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

Apply styling to all elements in td-content #1255

Merged
merged 1 commit into from
Oct 14, 2022
Merged

Conversation

the-kraljica
Copy link
Contributor

This PR is based off of PR 1017, as I could not get clarity on signing the Google CLA from my corporate GH account. I hope the contribution is still useful, my apologies for the overly long wait time.

As stated in #883 (and the original PR), tables were not being styled properly in <details><summary> blocks. I found the same issue to affect tables nested in list elements.
Removing > from the table entry enables proper styling to be applied to any table element that's a child of td-content.

As per the request in the original PR comments, I also removed the > selector from blockquote, ul li and ol li elements.

Copy link
Collaborator

@chalin chalin left a comment

Choose a reason for hiding this comment

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

LGTM, but would really want @geriom to give the final approval (and merge). Thanks all!

@chalin chalin self-assigned this Oct 7, 2022
@chalin chalin added the design/style Front-end site design / styling label Oct 7, 2022
Copy link
Collaborator

@geriom geriom left a comment

Choose a reason for hiding this comment

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

LGTM, Thank you!

@geriom geriom merged commit 1e7e329 into google:main Oct 14, 2022
anoadragon453 added a commit to matrix-org/matrix-spec that referenced this pull request Oct 27, 2022
Otherwise table-layout has no effect. I'm not sure why this broke,
possibly google/docsy#1255.
@anoadragon453
Copy link

Hello! This change caused <table> tags in our docsy site to have display: block instead of display: table, which broke some of our assumptions: https://github.com/matrix-org/matrix-spec/pull/1295/files#r1010523059.

I'm not sure if this was intended behaviour, but leaving a breadcrumb here if anyone would like to investigate on the docsy side :)

@chalin
Copy link
Collaborator

chalin commented Nov 11, 2022

@anoadragon453 - thank you for the in-depth analysis and feedback. Given that Docsy assumes that tables are responsive by default, yes this was intended. We're all looking forward to the Bootstrap upgrade. You may be right that after the upgrade your style change will be unnecessary. I'll add a note to the Bootstrap-upgrade issue as a reminder for us to check (added in opening comment).

@chalin
Copy link
Collaborator

chalin commented Feb 17, 2023

@chalin chalin mentioned this pull request May 4, 2023
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design/style Front-end site design / styling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants