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

refactor: avoid un-needed time chop-off #982

Closed
wants to merge 5 commits into from

Conversation

miles-grant-ibigroup
Copy link
Collaborator

Description:

Itinerary body was sometimes breaking time text when it shouldn't be. This PR resolves this. Will leave PR open for now to collect other styling regressions.

PR Checklist:

  • Does the code follow accessibility standards (WCAG 2.1 AA Compliant)?
  • Are all languages supported (Internationalization/Localization)?
  • Are appropriate Typescript types implemented?

@miles-grant-ibigroup miles-grant-ibigroup added the WIP Work in progress label Aug 23, 2023
@miles-grant-ibigroup miles-grant-ibigroup self-assigned this Aug 23, 2023
@binh-dam-ibigroup
Copy link
Collaborator

Just came across this one. Could setting the time-column component to have min-width: 60px and remove width: 60px do the trick regardless of the width of the containers?

@miles-grant-ibigroup
Copy link
Collaborator Author

Possibly! I know there are a lot of nesting styles going on here and then quite a bit of a11y changes happened as well so I'll have to spend some proper time with it to double check that doesn't introduce any regressions

@AdrianaCeric
Copy link
Member

I found that removing the width also did the trick. I think this is a lazy hack, but I've yet to find something else. Will look into it more, but let me know if you have any comments/suggestions!

@AdrianaCeric AdrianaCeric removed the WIP Work in progress label Sep 25, 2023
@AdrianaCeric AdrianaCeric marked this pull request as ready for review September 25, 2023 14:29
@miles-grant-ibigroup
Copy link
Collaborator Author

Removing styles is always a good thing!

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.

4 participants