Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding color to cursors #1248

Closed
wants to merge 32 commits into from
Closed

Conversation

wpkelso
Copy link
Member

@wpkelso wpkelso commented Mar 2, 2024

This does some basic work towards addressing #1065 , starting with the watch animation in both its standalone and ptr form. I liked nakotami's idea with the pencil coloring from here, so that has been added in as a starting point; other "real" cursors will eventually follow suit (zoom-in/zoom-out).

--EDIT--

This PR now encompasses not just the work addressing #1065, but also small modifications to many other cursor files to bring them more in-line with the rest of the set in addition to clarity improvements.

@wpkelso wpkelso marked this pull request as draft March 5, 2024 18:04
@wpkelso
Copy link
Member Author

wpkelso commented Mar 21, 2024

Current status of things

I tried not to touch the main bodies of cursors where possible, as well as keep similar icons on similar color-lines. I think it might be a good idea to keep the all-black set of cursors as an option available to the user for accessibility purposes.

@wpkelso wpkelso marked this pull request as ready for review March 21, 2024 17:45
Copy link
Member

@danirabbit danirabbit left a comment

Choose a reason for hiding this comment

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

What do you think about putting all the symbols for dnd-cursors on a colored circle like dnd-copy is? Some of these symbols are a bit harder to see in these versions. For example this feels like a clear step back for legibility:

Screenshot from 2024-03-21 11 55 13

I'm a little worried with making the working cursors blue that folks will naturally think this should be following their system accent color. Maybe there's something interesting we can do with rainbow colors here? Or maybe revert this for now and revisit

It looks like you accidentally included some changes for USB receiver icons in this branch

@wpkelso
Copy link
Member Author

wpkelso commented Mar 24, 2024

Not sure what happened with the USB changes, nothing actually changed there to my knowledge. However, I've reverted that file to main.

I reverted the watch cursors as well, I'll revisit them in the issue thread for a more thorough design exploration.

Changing to symbol-on-circle definitely helps with clarity for the dnd-cursors, although the size constraints meant I had to do some reworking of the symbols themselves.

@wpkelso wpkelso requested a review from danirabbit March 24, 2024 20:41
@wpkelso wpkelso marked this pull request as draft April 11, 2024 01:32
@wpkelso wpkelso marked this pull request as ready for review April 17, 2024 22:13
Copy link
Member

@danirabbit danirabbit left a comment

Choose a reason for hiding this comment

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

If the waiting cursors were their own branch, I would merge those today. I think that's an easy win. Those look great

For some of the other icons, I think there's more work to do here. Especially making sure we're consistent with the main icon set, using the colors and icons and styles already established there. I'll open a PR today with what I mean there

I'm not really fully convinced on the way the magnifier icons have been colored. I think generally updating separate families in separate branches might be better here so we can merge in what's working and consider/iterate on others separately

There's also still some issue with this PR touching the USB receiver icon, which it shouldn't

@wpkelso
Copy link
Member Author

wpkelso commented Apr 19, 2024

@danirabbit I think the original badges I was working with here were based on an earlier version of the HIG versus what you've done in #1250 , and I just failed to realize it.

I'll go ahead and split this up and close this PR. Hopefully that will resolve the issue with the USB receiver icon (I thought it had been unified with what was on main but perhaps that got pulled in before I had added the precommit on my end and thus wasn't cleaned up on main).

@wpkelso wpkelso closed this Apr 19, 2024
@wpkelso wpkelso deleted the cursor-update branch May 1, 2024 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants