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(website)!: use box-content for consistent column widths regardless of position #3730

Merged
merged 1 commit into from
Feb 24, 2025

Conversation

theosanderson
Copy link
Member

@theosanderson theosanderson commented Feb 21, 2025

preview URL: https://box-content.loculus.org/ebola-sudan/search? vs https://main.loculus.org/ebola-sudan/search?

Summary

We specifically add padding to the last column in the table for aesthetic reasons (last:pr-6), but this is problematic because we specify widths for each column but those widths include the padding and we don't know which column the user will choose to be last. Here we use box-content so that the specified width is just for the content, excluding any padding. This will likely require us to update all widths to take account of this.

All columns get a bit bigger on our current previews but I think that's ok as they are not tuned much on Loculus (they are more so on PPX)

Details of breaking changes

We may want to make all specified column widths on PPX 16 pixels narrower to account for the fact that padding is not longer being taken out

@theosanderson theosanderson added the preview Triggers a deployment to argocd label Feb 21, 2025
@theosanderson theosanderson changed the title fix(website): use border-box for consistent column widths regardless of position fix(website)!: use border-box for consistent column widths regardless of position Feb 21, 2025
@theosanderson theosanderson marked this pull request as draft February 21, 2025 16:08
@theosanderson theosanderson changed the title fix(website)!: use border-box for consistent column widths regardless of position fix(website)!: use box-content for consistent column widths regardless of position Feb 21, 2025
@theosanderson theosanderson marked this pull request as ready for review February 21, 2025 16:18
Copy link
Contributor

@anna-parker anna-parker left a comment

Choose a reason for hiding this comment

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

Im happy to approve this :-)

Copy link
Contributor

@corneliusroemer corneliusroemer left a comment

Choose a reason for hiding this comment

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

This sounds entirely reasonable but I'm not sure I understand the change - is it possible to show two screenshots or a Screencast that show the difference? I struggle to see them on mobile preview.

Not blocking, just curious

@corneliusroemer
Copy link
Contributor

Ah I think I see it, the last column breaks date into two lines in main but not on preview 😀

@theosanderson
Copy link
Member Author

👍 If you open the two links I posted in two tabs and switch between them you should notice the difference

@theosanderson theosanderson merged commit 3d3980a into main Feb 24, 2025
20 checks passed
@theosanderson theosanderson deleted the box-content branch February 24, 2025 13:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
preview Triggers a deployment to argocd
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants