-
Notifications
You must be signed in to change notification settings - Fork 152
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
Tailwind: Itinerary #4267
Tailwind: Itinerary #4267
Conversation
Storybook staging is available at https://kiwicom-orbit-tw-itinerary.surge.sh |
Size Change: -2.08 kB (0%) Total Size: 452 kB
ℹ️ View Unchanged
|
Deploying with Cloudflare Pages
|
bff7d00
to
4aba88a
Compare
.../Itinerary/Itinerary.ct.tsx-snapshots/visual-Itinerary-ItinerarySegment-1-Desktop-darwin.png
Outdated
Show resolved
Hide resolved
...nerary/Itinerary.ct.tsx-snapshots/visual-Itinerary-ItinerarySegment-open-1-Desktop-linux.png
Outdated
Show resolved
Hide resolved
packages/orbit-components/src/Itinerary/ItinerarySegment/ItineraryIcon.tsx
Show resolved
Hide resolved
This allows using the classes in other components that need it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All the issues I could catch seem resolved!
Seems the visual testing are failing due to the change in the z-index, but it looks minor.
Feel free to add RTL visual testing if you like, I guess we could have detected the issue with left/right vs start/end.
Huge effort, good job!
some visual tests were updated because some bugs were actually fixed during this migration
I did not find an obvious way to test it on a very quick look, so I guess it might be worth a task in the future for all components. I'll skip it for now |
Itinerary component migrated to Tailwind. A few implementation details about this specific component:
zIndex
on ItineraryIcon was removed.For Tailwind migrations:
tailwind-migration-status.yaml
file has been updated with the migrated component(s).Closes FEPLT-1708