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

fix(home view): use correct Quick Links data #6450

Merged
merged 2 commits into from
Feb 20, 2025

Conversation

anandaroop
Copy link
Member

@anandaroop anandaroop commented Feb 19, 2025

https://artsyproduct.atlassian.net/browse/ONYX-1554

Update to artsy/eigen#11528

See: Slack comment

Updates Quick Links to use correct pill names, destinations, and icons.

This will result in the following change in Eigen:

Before After

Note that two of the required pills are still missing:

  • Medium which has a complication due to the unconventional way in which Eigen navigates to it.
  • Featured Fairs which has yet to be merged into Eigen

Those are included but commented out here. They can be updated as a follow-up.

@anandaroop anandaroop self-assigned this Feb 19, 2025
@anandaroop anandaroop force-pushed the anandaroop/ONYX-1484-pill-icons branch from 7055bdf to b9a035b Compare February 19, 2025 23:12
@anandaroop anandaroop force-pushed the anandaroop/ONYX-1484-pill-icons branch from b9a035b to 9d10ac4 Compare February 19, 2025 23:12
@artsy artsy deleted a comment from artsy-peril bot Feb 19, 2025
Comment on lines +49 to +52
// TODO: enable once navigation is sorted out
// {
// title: "Medium",
// href: "/collections-by-category/Medium",
Copy link
Member Author

Choose a reason for hiding this comment

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

This one is tough! I tried to build the correct href, but failed because of the unusual way in which we are routing from the home view to those screens. This will need some attention, but we can also merge here and follow up.

dariakoko
dariakoko previously approved these changes Feb 20, 2025
MounirDhahri
MounirDhahri previously approved these changes Feb 20, 2025
},

// TODO: enable once eigen/11476 is merged
Copy link
Member

Choose a reason for hiding this comment

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

we might need at some point to support some versioning here - definitely not needed for now

Copy link
Member Author

Choose a reason for hiding this comment

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

@MounirDhahri yeah that's a good point, worth some discussion.

@anandaroop anandaroop dismissed stale reviews from MounirDhahri and dariakoko via f5005a1 February 20, 2025 18:23
@anandaroop
Copy link
Member Author

Cool, I've resolved those analytics todos, and will merge this now as well

@anandaroop anandaroop added the Squash On Green A label to indicate that Peril should squash-merge this PR when all statuses are green label Feb 20, 2025
@artsy-peril artsy-peril bot merged commit 558d42f into main Feb 20, 2025
4 checks passed
@artsy-peril artsy-peril bot deleted the anandaroop/ONYX-1484-pill-icons branch February 20, 2025 18:27
@artsy-peril artsy-peril bot mentioned this pull request Feb 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Squash On Green A label to indicate that Peril should squash-merge this PR when all statuses are green
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants