Skip to content

Conversation

@luckyklyist
Copy link
Contributor

@luckyklyist luckyklyist commented Sep 14, 2023

Fix Confusing Icons #2255

Changes Made

I've addressed the issue of confusing icons that occurred due to the Sass hover styles. To enhance the user experience, I reorganized the code related to hover-specific styles. Now, users can clearly see whether they've added a sketch without needing to hover over the button.

Screen.Recording.2023-09-14.at.2.52.21.PM.mov

I have verified that this pull request:

  • has no linting errors (npm run lint)
  • has no test errors (npm run test)
  • is from a uniquely-named feature branch and is up to date with the develop branch.
  • is descriptively named and links to an issue number, i.e. Fixes #123

@luckyklyist
Copy link
Contributor Author

@lindapaiste can you review this pr ?

@lindapaiste
Copy link
Collaborator

lindapaiste commented Sep 22, 2023

I think this is an improvement so I should probably merge it as-is. Just need to load up the branch and run it myself.

Since we are no longer showing different icons on hover the CSS could be cleaned up further, if you want to. We can use the JS to return only the one matching icon and remove all of the display: none; and display: inline-block; switching from the CSS, rather than having both in the HTML and using CSS to hide one.

There are things which are still weird about this QuickAddList which can be handled in future PRs. We have an "are you sure?" dialog before removing a sketch via the normal list, but we don't have any confirmation here. And also there's an aria-label="Descending" which make no sense 🙃. There's a correct label on the parent button so it should be aria-hidden="true" instead. But those things aren't related to the confusing icons.

lindapaiste
lindapaiste previously approved these changes Sep 22, 2023
Copy link
Collaborator

@lindapaiste lindapaiste left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is definitely an improvement! It still feels a bit confusing though, especially because the "+" and "x" icons are so visually similar. Do you think it would be better if we used the ✔️ instead of the "x" for items in the collection?

@luckyklyist
Copy link
Contributor Author

I have changed the + to ✔️ .
Screenshot 2023-09-23 at 11 10 42 PM

@raclim raclim added Area: Design For UI/UX design updates, proposals, or feedback Enhancement Improvement to an existing feature labels Jan 26, 2024
Copy link
Collaborator

@raclim raclim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your work on this! :)

@raclim raclim merged commit 259ad33 into processing:develop Jun 3, 2024
@raclim raclim mentioned this pull request Jun 3, 2024
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: Design For UI/UX design updates, proposals, or feedback Enhancement Improvement to an existing feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants