Conversation
|
View your CI Pipeline Execution ↗ for commit ccbb4ff. ☁️ Nx Cloud last updated this comment at |
📬Published Alpha Packages:@codecademy/gamut@57.7.3-alpha.ccbb4f.0 |
|
🚀 Styleguide deploy preview ready! |
dreamwasp
left a comment
There was a problem hiding this comment.
for accessibility, our Pagination'Buttons' are polymorphic, so when they are provided a href they render as an a element (you'll see this in the Gamut page for Pagination if you inspect the isNavigation example). i can help with the framer-motion upgrades, the types can get pretty wacky with our more flexible components.
i will need to look into this more, but i find it hard to believe that none of our pagination components are used as navigation and this is likely just developer oversight... in the meantime, i'll cut a ticket to make the isNavigation no longer optional so folks are forced to think about it when using the component
|
Thanks @dreamwasp ! Yep, that's a good point. Maybe when developers have to think about which one to use, we'll discover places where we should be using I had assumed that the type checking change after upgrading framer motion upgrade was an improvement, but perhaps it actually is a bug/failure to pick up on the |
Overview
When attempting to upgrade framer-motion, improved type checking alerted us to the fact that we were trying to set
hrefon a button when usingisNavigationforPagination. Upon searching the monorepo and monolith, no usage of theisNavigationprop was found. Thus, it seems reasonable to upblock the upgrade by just removing this feature.PR Checklist
Related to designs:Related to JIRA ticket: [ABC-123]This PR includes unit tests for the code changeThis PR includes testing instructions tests for the code changePR Links and Envs