the-hideout / tarkov-dev

The official site for tarkov.dev - A web app to track item prices, view trader barters, quests, maps, and much more!
https://tarkov.dev
MIT License
130 stars 48 forks source link

[Enhancement] Footer Changes #915

Closed DysektAI closed 2 months ago

DysektAI commented 2 months ago

Edited the footer layout

Description 🗒️

Changes -Removed "Escape from Tarkov Dev tracker" seems inactive and rarely necessary unless this is required I don't see a use for it -Removed the commented out link to "API Users" page +Added "Tarkov Monitor" as a resource linked to the github since its not listed anywhere ~Re-ordered the Resources and External Sources to be more stylish

(Reverted) ~Discord: Simplified to just use a shields.io img/button that shows users online count (Still requires widget to be enabled on server)~ ~X/Twitter: Also changed to use shields.io img/button to view X/Twitter~ ~Removed the " : " in the "Made with love by"~

Examples 📸

Before image After image

Related Issues 🔗

914 (Does not fix the issue but its planned)

GrantBirki commented 2 months ago

.deploy to development

github-actions[bot] commented 2 months ago

⚠️ Cannot claim deployment lock

Sorry GrantBirki, the development environment deployment lock is currently claimed by Razzmatazzz

Lock Details 🔒

The current lock has been active for 5d:6h:26m:20s

If you need to release the lock, please comment .unlock development

Razzmatazzz commented 2 months ago

.unlock development

github-actions[bot] commented 2 months ago

🔓 Deployment Lock Removed

The development deployment lock has been successfully removed

DysektAI commented 2 months ago

Discord: Simplified to just use a shields.io img/button that shows users online count (Still requires widget to be enabled on server) X/Twitter: Also changed to use shields.io img/button to view X/Twitter

These are some significant changes and I would like to keep them as they were for now, revert please.

Removed the " : " in the "Made with love by"

The use of a colon here is justified as it introduces a list of contributors, so please revert it back too :)

Reverted now back to original formatting to comply with your request 👍

Shebuka commented 2 months ago

Discord: Simplified to just use a shields.io img/button that shows users online count (Still requires widget to be enabled on server) X/Twitter: Also changed to use shields.io img/button to view X/Twitter

These are some significant changes and I would like to keep them as they were for now, revert please.

Removed the " : " in the "Made with love by"

The use of a colon here is justified as it introduces a list of contributors, so please revert it back too :)

Reverted now back to original formatting to comply with your request 👍

I just looked at the changes and the discord iframe is still missing

DysektAI commented 2 months ago

I'm confused... the reason for this PR mainly was to fix the issue originally opened #914 unless maybe you know a way to fix the CORS error without removing the iFrame / making sure no one has a default profile picture / avatar and finding a way to fix the discord widget from extending off the page, I am all for it but currently there is no known resolutions hence why I said in the OP above: (I did search for others who had this issue but it seems like the widget for discord was made in 2016 and the most recent posts I found were in 2022 with no answers that are similar to this)

Not sure why the default profile avatars for discord are throwing CORS errors so I assume it's best to just remove it / replace it (Also when hovered it extended past the bottom of the screen)

image

DysektAI commented 2 months ago

There is also already multiple references to the discord I attempted to use the shields.io as a replacement for the "online users" as that is nice to see but you asked to revert / remove that change.

Shebuka commented 2 months ago

There is also already multiple references to the discord I attempted to use the shields.io as a replacement for the "online users" as that is nice to see but you asked to revert / remove that change.

You replaced the iframe with the image in the PR, but using iframe is the current and only official way to embed a discord widget in the site.

Also, it does work and everything is loading fine so I want to keep using it.

Shebuka commented 2 months ago

I'm confused... the reason for this PR mainly was to fix the issue originally opened #914 unless maybe you know a way to fix the CORS error without removing the iFrame / making sure no one has a default profile picture / avatar and finding a way to fix the discord widget from extending off the page, I am all for it but currently there is no known resolutions hence why I said in the OP above: (I did search for others who had this issue but it seems like the widget for discord was made in 2016 and the most recent posts I found were in 2022 with no answers that are similar to this)

Not sure why the default profile avatars for discord are throwing CORS errors so I assume it's best to just remove it / replace it (Also when hovered it extended past the bottom of the screen)

image

As of 15 October 2023 they are still actively supporting the widget: https://github.com/discord/discord-api-docs/issues/6447 I sincerely do not understand why you think the widget is bad?

Its expanding and changing footer height with animation is not ideal, but that can be solved with layout updates, not by removing the widget.

DysektAI commented 2 months ago

I'm confused... the reason for this PR mainly was to fix the issue originally opened #914 unless maybe you know a way to fix the CORS error without removing the iFrame / making sure no one has a default profile picture / avatar and finding a way to fix the discord widget from extending off the page, I am all for it but currently there is no known resolutions hence why I said in the OP above: (I did search for others who had this issue but it seems like the widget for discord was made in 2016 and the most recent posts I found were in 2022 with no answers that are similar to this)

Not sure why the default profile avatars for discord are throwing CORS errors so I assume it's best to just remove it / replace it (Also when hovered it extended past the bottom of the screen)

image

As of 15 October 2023 they are still actively supporting the widget: discord/discord-api-docs#6447 I sincerely do not understand why you think the widget is bad?

Its expanding and changing footer height with animation is not ideal, but that can be solved with layout updates, not by removing the widget.

I can give a list of reasons why I don't think the discord widget is good

  1. It expands and causes weird issues where users have to scroll awkwardly through a list of users to even see or access the "join server" button and expands the footer to over half of the monitor on desktop.
  2. The discord icon does not link to the server, it links directly to discords website which users will most likely click on if they don't scroll.
  3. Do users need to see the voice channels and users in the discord on the page before clicking join server?

If you insist on keeping the discord widget I would almost be more excited to create a customized version of it than using the default widget layout with something like the one below

image

Just because something "works and everything is loading fine" does not mean its the best option.

If you want to keep everything as is that's ok just edit my PR or close it I'll work on a custom widget if it is a must have when I have time.

Razzmatazzz commented 2 months ago

I agree the current Discord widget has some issues (especially on mobile). It might make sense to just have a discord image and join link. But I could be overlooking some of the usefulness of the current widget.

Blightbuster commented 2 months ago

My 5 cents: While the current discord widget is the official one, the two negatives @lyricalcasanova outlined above heavily outway that. Id be onboard with either disabling the folding and make the click action redirect to our join link instead of discords website or just using shields.io at that point. Im using shields.io for many of my repos as well and only ever had good experiences with it so far.

Shebuka commented 2 months ago

I'm confused... the reason for this PR mainly was to fix the issue originally opened #914 unless maybe you know a way to fix the CORS error without removing the iFrame / making sure no one has a default profile picture / avatar and finding a way to fix the discord widget from extending off the page, I am all for it but currently there is no known resolutions hence why I said in the OP above: (I did search for others who had this issue but it seems like the widget for discord was made in 2016 and the most recent posts I found were in 2022 with no answers that are similar to this)

Not sure why the default profile avatars for discord are throwing CORS errors so I assume it's best to just remove it / replace it (Also when hovered it extended past the bottom of the screen)

As of 15 October 2023 they are still actively supporting the widget: discord/discord-api-docs#6447 I sincerely do not understand why you think the widget is bad? Its expanding and changing footer height with animation is not ideal, but that can be solved with layout updates, not by removing the widget.

I can give a list of reasons why I don't think the discord widget is good

  1. It expands and causes weird issues where users have to scroll awkwardly through a list of users to even see or access the "join server" button and expands the footer to over half of the monitor on desktop.
  2. The discord icon does not link to the server, it links directly to discords website which users will most likely click on if they don't scroll.
  3. Do users need to see the voice channels and users in the discord on the page before clicking join server?

If you insist on keeping the discord widget I would almost be more excited to create a customized version of it than using the default widget layout with something like the one below

Just because something "works and everything is loading fine" does not mean its the best option.

If you want to keep everything as is that's ok just edit my PR or close it I'll work on a custom widget if it is a must have when I have time.

It's not that "Just because something "works and everything is loading fine" does not mean its the best option." You are pointing out some valid critiques now, but they were not in your initial PR where the main reason for replacement are CORS errors which are IMO harmless as they are thrown only for default avatars and the rest works fine. But you were not providing an apples-to-apples replacement, that's why I've asked it to be reverted as losing functionality for some harmless errors is not a solution.

My first glance comparison was:

shields.io: Pros:

Cons:

Discord widget: Pros:

when expanded:

Cons:

No call to action in your shields.io implementation is a major stopper for me (it's also why I didn't like shields.io for X). As for other pros missing, I can deal with it if for others it's ok.

DysektAI commented 2 months ago
Previous Replies > > > > I'm confused... the reason for this PR mainly was to fix the issue originally opened #914 unless maybe you know a way to fix the CORS error without removing the iFrame / making sure no one has a default profile picture / avatar and finding a way to fix the discord widget from extending off the page, I am all for it but currently there is no known resolutions hence why I said in the OP above: (I did search for others who had this issue but it seems like the widget for discord was made in 2016 and the most recent posts I found were in 2022 with no answers that are similar to this) > > > > > Not sure why the default profile avatars for discord are throwing CORS errors so I assume it's best to just remove it / replace it (Also when hovered it extended past the bottom of the screen) > > > > > > > > > As of 15 October 2023 they are still actively supporting the widget: [discord/discord-api-docs#6447](https://github.com/discord/discord-api-docs/issues/6447) I sincerely do not understand why you think the widget is bad? > > > Its expanding and changing footer height with animation is not ideal, but that can be solved with layout updates, not by removing the widget. > > > > > > I can give a list of reasons why I don't think the discord widget is good > > > > 1. It expands and causes weird issues where users have to scroll awkwardly through a list of users to even see or access the "join server" button and expands the footer to over half of the monitor on desktop. > > 2. The discord icon does not link to the server, it links directly to discords website which users will most likely click on if they don't scroll. > > 3. Do users need to see the voice channels and users in the discord on the page before clicking join server? > > > > If you insist on keeping the discord widget I would almost be more excited to create a customized version of it than using the default widget layout with something like the one below > > Just because something "works and everything is loading fine" does not mean its the best option. > > If you want to keep everything as is that's ok just edit my PR or close it I'll work on a custom widget if it is a must have when I have time.

It's not that "Just because something "works and everything is loading fine" does not mean its the best option." You are pointing out some valid critiques now, but they were not in your initial PR where the main reason for replacement are CORS errors which are IMO harmless as they are thrown only for default avatars and the rest works fine. But you were not providing an apples-to-apples replacement, that's why I've asked it to be reverted as losing functionality for some harmless errors is not a solution.

My first glance comparison was:

shields.io: Pros:

  • Simple
  • Users count

Cons:

  • No call to action

I find the Discord 300 Online more of a call to action than a generic discord widget, that users get lost just trying to find the "join server" button because it overflows off the screen then users have to scroll down users list to the bottom battling the on hover transition. Not to mention we have a true call to action already in the footer... image

  • Styling is not DIscord's

Styling in Shields is subjective, I simply used the color pallet that I thought looks clean if needed discord colors can be used for that also

  • No list of online users

Do you really need to see the online users list in the discord before you join? I can't remember a single time I checked a user list to see if I wanted to join a discord...

Discord widget: Pros:

  • Users count

User count - Online Users is also in Shields.io as one of your pros above image

  • Styling distinctive to Discord

See above (Unless you mean the widget itself)

when expanded:

  • List of online users

See above

  • Call to action

See above

  • Voice channels (opinable)

Same argument as the users list I have never worried about how many users are in the voice channels especially for a server centered around developing and tarkov news/updates (This would be different if the server was centered around just gaming)

Cons:

  • Animation on expansion (fixable)
  • Voice channels (opinable)
  • Call to action only visible when expanded

No call to action in your shields.io implementation is a major stopper for me (it's also why I didn't like shields.io for X). As for other pros missing, I can deal with it if for others it's ok.

With all respect, @Shebuka it seems we can't come to a solid agreement. You do have the option to contribute your own code / changes that you would like to see if you feel strongly about anything. From the consensus so far, it seems like you may be the only one not okay with removing this generic widget I would understand your argument if you created the code being removed, but it's simply a widget that can always be moved to somewhere else or added later if a better option is presented.

I'm tired of debating over 32 changes, at this point unless there is truly a valid argument to make I will leave it up to the maintainers to decide what they want to do with this PR either Close, Edit, or Merge.

Shebuka commented 2 months ago

Oh man, just wow... In the last comment, I've explained to you the reason behind my INITIAL request to rollback... I thought it was clear with using the past tense...

Not to mention we have a true call to action already in the footer...

Where is a call to action here? (your screen) image

YOU said that

I'll work on a custom widget

and Im saying ok if it has a call to action and is stylized like DIscord as YOU showed in the screenshot.

image

Where does even

With all respect, @Shebuka it seems we can't come to a solid agreement.

come from???

Razzmatazzz commented 2 months ago

Seems like this discussion is causing some frustration for multiple people. Let's not lose sight of the fact that we all have the common goal of making the website as good as it can be to be as useful to the community as possible. Yes, we will sometimes have differences of opinion on what makes the website the best, but we should be able to discuss those differences respectfully and reach a consensus on the best way to move forward.

My two cents is that the current discord widget causes some annoying behavior (particularly on mobile). The shields.io solution seems like a good substitute as it provides a recognizable join link plus the current user count. While it does lack the ability to see who is online, voice channels, etc., I don't view those features as being vital (especially when crammed into the website footer).

DysektAI commented 2 months ago

Oh man, just wow... In the last comment, I've explained to you the reason behind my INITIAL request to rollback... I thought it was clear with using the past tense...

Not to mention we have a true call to action already in the footer...

Where is a call to action here? (your screen) image

YOU said that

I'll work on a custom widget

and Im saying ok if it has a call to action and is stylized like DIscord as YOU showed in the screenshot.

image

Where does even

With all respect, @Shebuka it seems we can't come to a solid agreement.

come from???


The call to action is the shields.io with a count of the online users, this causes curiosity from the users to wonder what is in the discord which makes them want to click it.

The requests to rollback was done as you requested with the exception of the discord widget as you did not mention the widget in your original request here:

Discord: Simplified to just use a shields.io img/button that shows users online count (Still requires widget to be enabled on server) X/Twitter: Also changed to use shields.io img/button to view X/Twitter

These are some significant changes and I would like to keep them as they were for now, revert please.

Removed the " : " in the "Made with love by"

The use of a colon here is justified as it introduces a list of contributors, so please revert it back too :)


Maybe there was some misunderstanding from the above comment but I see no reference in this comment anywhere specifying the discord widget not being removed simply just asking "revert please." with my change points referenced about the shields.io for x/twitter and discord this could not be further from clearly communicating in the future please provide more details, examples, or code so it's more clear.

I did as you requested which is the current state of this PR, then you asked where the widget is? I told you I could make one later on, if required or you can contribute also. You then continued to argue that its not valid to remove the widget, so basically should I not contribute and revert all the changes? What would make you happy? I'm a bit confused, maybe if you propose some code or a demonstration of what your changes would look like then I would understand better I would love to see what your final product looks like and how it functions compared to what I proposed.

DysektAI commented 2 months ago

Also please enlighten me maybe my understanding of a "Call To Action" is wrong I thought a call to action was something that provokes people to click or find interest in doing something like a button / link? Is the button that says "Discord 283 Online" with a link to the Discord Server not a Call To Action?

Maybe it has to be a big bright button that follows none of the websites color scheme and jumps off the page or says "CLICK TO JOIN THE DISCORD" but I'm not sure because "Call To Action" in website design is defined as: A prompt on a website that tells the user to take some specified action. A call to action is typically written as a command or action phrase, such as 'Sign Up' or 'Buy Now' and generally takes the form of a button or hyperlink.

Shebuka commented 2 months ago

Maybe there was some misunderstanding, let's reset the conversation.


On this point:

I see no reference in this comment anywhere specifying the discord widget not being removed simply just asking "revert please." with my change points referenced about the shields.io for x/twitter and discord this could not be further from clearly communicating in the future please provide more details, examples, or code so it's more clear.

Briefly:

I hope this helps understand the whole conversation flow and clears the confusion.


About what is a "Call To Action":

As you quoted it is any form of 'visual' message (a text, a button, an arrow) that invites the user to do something.

An image with "Discord 283 Online" is a statement of fact, like "build: passing" (see below). By look and feel it has all the functions of an image, no user will think it's a button without hovering the mouse over it (can't be done on mobile), and even after that they don't know what it does (URL in the status bar is not telling much), so it's clearly not a "Call To Action".

So, like what you showed is a perfect component with a clear call to action: image

p.s. I never saw use of shileds outside of github and other related coding websites, so to general Tarkov website users it's just an image, as it is the build-passing-brightgreen

DysektAI commented 2 months ago

Note: Accidentally merged the wrong branch (#911) force pushed to revert the merge, I would normally just add a revert commit but this was necessary

@Shebuka and @Razzmatazzz I misunderstood what was exactly wanted / expected, but I feel like I was misunderstood also.

I have reverted the change and included the Discord Widget until there's an acceptable replacement for it as Shebuka is right it's best to have both options until then.

Razzmatazzz commented 2 months ago

.deploy to development

github-actions[bot] commented 2 months ago

Deployment Triggered 🚀

Razzmatazzz, started a branch deployment to development

You can watch the progress here 🔗

Branch: 2a93717c1e32f51bf301252493e33b56f122aba4

github-actions[bot] commented 2 months ago

Deployment Results ✅

Razzmatazzz successfully deployed branch 2a93717c1e32f51bf301252493e33b56f122aba4 to development

Show Results https://44b24540.tarkov-dev.pages.dev
Shebuka commented 2 months ago

.deploy

github-actions[bot] commented 2 months ago

Deployment Triggered 🚀

Shebuka, started a branch deployment to production

You can watch the progress here 🔗

Branch: 6e42b4a2e58decf6ac8c55e7b1e28f5ba878a3ad

github-actions[bot] commented 2 months ago

Deployment Results ✅

Shebuka successfully deployed branch 6e42b4a2e58decf6ac8c55e7b1e28f5ba878a3ad to production

Environment URL: tarkov.dev