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

Add custom cursor icons back #3649

Merged
merged 9 commits into from
Mar 23, 2023
Merged

Add custom cursor icons back #3649

merged 9 commits into from
Mar 23, 2023

Conversation

snowystinger
Copy link
Member

@snowystinger snowystinger commented Oct 14, 2022

Closes

Adds custom cursor icons back
We addressed parcel issue with a workaround to prevent Parcel from overwriting its assets that it loads into both browser and node environment. See parcel-bundler/parcel#5985 (comment) for more information

✅ Pull Request Checklist:

  • Included link to corresponding React Spectrum GitHub Issue.
  • Added/updated unit tests and storybook for this change (for new code or code which already has tests).
  • Filled out test instructions.
  • Updated documentation (if it already exists for this component).
  • Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

🧢 Your Project:

@adobe-bot
Copy link

@snowystinger snowystinger marked this pull request as ready for review December 15, 2022 03:06
@snowystinger snowystinger mentioned this pull request Dec 16, 2022
61 tasks
@snowystinger snowystinger added the small review Easy to review PR label Jan 5, 2023
Copy link
Member

@LFDanLu LFDanLu left a comment

Choose a reason for hiding this comment

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

Hm, something up with the docs build not sure what, but I did verify the the custom icons work locally

@snowystinger
Copy link
Member Author

Hm, something up with the docs build not sure what, but I did verify the the custom icons work locally

Nooooo, that's the original issue that caused me to revert the custom icons. Unfortunately it's a race condition, so I thought when several builds passed that the parcel update had fixed it, but it looks like that was too optimistic.

@rspbot
Copy link

rspbot commented Mar 22, 2023

Copy link
Member

@ktabors ktabors left a comment

Choose a reason for hiding this comment

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

Fantastic!

Copy link
Member

@LFDanLu LFDanLu left a comment

Choose a reason for hiding this comment

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

LGTM, there is a grey outline around the cursor in Safari but should be fine IMO

@snowystinger
Copy link
Member Author

snowystinger commented Mar 23, 2023

following up on the Safari scaled-up issue separately

@rspbot
Copy link

rspbot commented Mar 23, 2023

@rspbot
Copy link

rspbot commented Mar 23, 2023

## API Changes

unknown top level export { type: 'identifier', name: 'Column' }
unknown top level export { type: 'identifier', name: 'Column' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }

@snowystinger snowystinger merged commit 3c0d990 into main Mar 23, 2023
@snowystinger snowystinger deleted the revert-fix-build branch March 23, 2023 22:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
small review Easy to review PR
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

7 participants