Skip to content

Conversation

@gabalafou
Copy link
Collaborator

@gabalafou gabalafou commented May 27, 2024

This PR makes a few changes to the _tables.scss file:

  • Matches our table alignment classes with an update to MyST from 2021 (version 0.16.0)
  • Removes the redundant and useless display: table and overflow: auto rules
  • Updates a comment

Fixes #1804.

Copy link
Collaborator Author

@gabalafou gabalafou left a comment

Choose a reason for hiding this comment

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

self review

Comment on lines +33 to +41
&.text-left {
text-align: left;
}

&.text-align\:right {
&.text-right {
text-align: right;
}

&.text-align\:center {
&.text-center {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These classes were changed in executablebooks/MyST-Parser#450

Comment on lines -9 to -11
display: table;
overflow: auto;

Copy link
Collaborator Author

@gabalafou gabalafou May 27, 2024

Choose a reason for hiding this comment

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

A bit of flip-flop history here:

As I discuss in issue #1824, it's probably a bad idea to change the display property of a table element since that might strip it of its semantics. And there's no need to explicitly put display: table on the table element, because that's what browsers assume.

As for the overflow rule, it only made sense when the table had display: block, as I discuss in PR #1827.

So to reduce head-scratching for future devs, I'm removing both rules.

@dbitouze
Copy link
Contributor

dbitouze commented May 27, 2024

This PR makes a few changes to the _tables.scss file:

Why a so old version of MyST?

Fixes #1804.

Thanks!

@Carreau
Copy link
Collaborator

Carreau commented May 27, 2024

Why a so old version of MyST?

I think what was meant are "this changes has been introduced in myst 0.16 in 2021, so are not recent, therefore safe", not "we want to align to an old version of myst".

@gabalafou
Copy link
Collaborator Author

gabalafou commented May 27, 2024

I think what was meant are "this changes has been introduced in myst 0.16 in 2021, so are not recent, therefore safe", not "we want to align to an old version of myst".

Correct. That's my justification for not leaving a backwards-compatibility layer.

@Carreau Carreau merged commit aabe57d into pydata:main May 27, 2024
@gabalafou gabalafou deleted the left-right-start-end-table branch May 27, 2024 17:01
ivanov pushed a commit to ivanov/pydata-sphinx-theme that referenced this pull request Jun 5, 2024
This PR makes a few changes to the _tables.scss file:

- Matches our table alignment classes with an [update to MyST from 2021](executablebooks/MyST-Parser#450) (version [0.16.0](https://github.com/executablebooks/MyST-Parser/releases/tag/v0.16.0))
- Removes the redundant and useless `display: table` and `overflow: auto` rules
- Updates a comment

Fixes pydata#1804.
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.

Table cell right alignment is wrong

3 participants