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

Replace table with grid #11

Closed
wants to merge 1 commit into from
Closed

Conversation

ChHecker
Copy link

@ChHecker ChHecker commented Nov 27, 2024

In print-index, you used table. This is problematic for two reasons:

  1. In the documentation, it says to use grid in your case. This also helps with accessibility.
  2. When I restyle tables in my program using set rules, it also changes the acrostiche index.

grid is plug-and-play, so I just replaced it.

@Grisely
Copy link
Owner

Grisely commented Nov 27, 2024

Thank you for your PR.
I am perfectly open to changing that if it improves anything. I will give it a look soon.
For the first point I definitely agree that anything that improves accessibility should be prioritized.
Regarding your second point about the styles, would it not be the same problem with a grid? If you style grids with a set rule it will be applied to the acronyms index grid too. Let me know if I missed something there.

@ChHecker
Copy link
Author

Usually, grids are used for stacking content horizontally and as such, you usually don't change their layout globally. tables, however, are frequently used in figures and therefore often need to have consistent formatting throughout a document, which can be easily enforced with set rules.

@Grisely
Copy link
Owner

Grisely commented Nov 30, 2024

I merge the changes in main after rebasing your branch on a newer version of main with a new version number to prepare the next version of the package. I will close the PR instead of merging. Thank you for your contribution. It will be included in the next version.

@Grisely Grisely closed this Nov 30, 2024
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