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

BB-714: Fix search results page on Safari #987

Merged
merged 3 commits into from
Jun 5, 2023

Conversation

faraz16iqbal
Copy link
Contributor

BB-714

Problem

The width of tables on the Webkit-based browser are broken.

The width of the columns is defined using Bootstrap CSS classes, which is not working on Webkit.

Solution

Setting the column sizes using the width attribute in percentages solves the issue.

Areas of Impact

All tables rendered on the Bookbrainz website that use the col-md class name to specify the width.

@faraz16iqbal
Copy link
Contributor Author

@MonkeyDo there were some other tables that were using the col-md class name to specify the width. I have updated all of them under this PR

Copy link
Member

@MonkeyDo MonkeyDo left a comment

Choose a reason for hiding this comment

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

It think this should all be done using the width attribute rather than inline style, as suggested in the ticket:

<tr width="16%"> instead of <tr style="width:16%">

@faraz16iqbal faraz16iqbal requested a review from MonkeyDo May 21, 2023 05:15
Copy link
Member

@MonkeyDo MonkeyDo left a comment

Choose a reason for hiding this comment

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

Thanks, this looks a whole lot better already!

I did a bit of testing after putting the PR up on test.bookbrainz, and found the profile page looking like this:

image

Not sure what's happening there but I'll leave it with you to investigate.
If you need help testing with Safari I can help out (otherwise there's also BrowserStack that I sometimes use for browser testing)

@faraz16iqbal
Copy link
Contributor Author

Thanks, this looks a whole lot better already!

I did a bit of testing after putting the PR up on test.bookbrainz, and found the profile page looking like this:

image Not sure what's happening there but I'll leave it with you to investigate. If you need help testing with Safari I can help out (otherwise there's also BrowserStack that I sometimes use for browser testing)

I think its because of the dt and dh tags not supporting the width attribute. I think reverting this will solve the problem

@faraz16iqbal
Copy link
Contributor Author

@MonkeyDo this should be good to go now!

@faraz16iqbal faraz16iqbal requested a review from MonkeyDo May 31, 2023 19:58
Copy link
Member

@MonkeyDo MonkeyDo left a comment

Choose a reason for hiding this comment

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

Thank you !

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.

2 participants