Skip to content

Comments

Add modern components from the Sphinx ecosystem#378

Merged
amotl merged 8 commits intomainfrom
amo/modernize-nt
Jul 21, 2023
Merged

Add modern components from the Sphinx ecosystem#378
amotl merged 8 commits intomainfrom
amo/modernize-nt

Conversation

@amotl
Copy link
Member

@amotl amotl commented Jul 13, 2023

About

This patch adds a few modern components from the Sphinx ecosystem to the theme, and demonstrates how they are used. Thanks to all the authors for your excellent work.

Preview

The preview of the demonstration pages can be inspected at RTD: CrateDB docs theme » Preview. There is a dedicated section demonstrating how to use the corresponding modern Sphinx components, elements, and styles.

https://crate-docs-theme--378.org.readthedocs.build/en/378/#modernized

@amotl amotl force-pushed the amo/modernize-nt branch from f427ccf to dccccc6 Compare July 13, 2023 21:08
@amotl amotl changed the base branch from main to amo/rtd-pr-build July 13, 2023 21:09
@amotl amotl requested review from hlcianfagna, matkuliak and seut July 13, 2023 21:32
@matkuliak
Copy link
Contributor

Hello @amotl this is great, thank you!
I noticed you suggest removal of sphinx_copybutton. Why is that? Is it part of some of the newly added components?

@amotl
Copy link
Member Author

amotl commented Jul 14, 2023

I noticed you suggest removal of sphinx_copybutton.

That would not have been intended. Please add an inline comment on the PR where you discovered this spot.

@amotl amotl force-pushed the amo/modernize-nt branch from dc1d258 to 225ed45 Compare July 14, 2023 08:57
Copy link
Member

@seut seut left a comment

Choose a reason for hiding this comment

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

Great! 👍

@amotl amotl force-pushed the amo/rtd-pr-build branch 2 times, most recently from d86eb2a to 1ba3b76 Compare July 14, 2023 23:22
Base automatically changed from amo/rtd-pr-build to main July 15, 2023 00:00
@amotl amotl force-pushed the amo/modernize-nt branch from 225ed45 to 1290b3a Compare July 15, 2023 00:03
@amotl amotl requested a review from matkuliak July 15, 2023 00:04
@amotl amotl marked this pull request as ready for review July 15, 2023 00:18
Copy link

@hlcianfagna hlcianfagna left a comment

Choose a reason for hiding this comment

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

Comment on lines 32 to 34
### Inline quotes / citations

> This text should be quoted.
Copy link
Member Author

@amotl amotl Jul 17, 2023

Choose a reason for hiding this comment

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

It looks like the Markdown/MyST markup instruction to quote/cite text does not render properly yet. /cc @msbt

image

-- https://crate-docs-theme--378.org.readthedocs.build/en/378/myst-overview.html#inline-quotes-citations

Copy link
Contributor

Choose a reason for hiding this comment

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

@amotl do we have an example on how that's supposed to look like? We probably need to remove some css from the original theme to make this work

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Exactly. It is written like

> Franz jagt im komplett verwahrlosten Taxi quer durch Bayern.

and usually renders like

Franz jagt im komplett verwahrlosten Taxi quer durch Bayern.

Copy link
Contributor

@matkuliak matkuliak Jul 17, 2023

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Looks good, I think you found the right spot, thanks. 🙇

Copy link
Contributor

Choose a reason for hiding this comment

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

@matkuliak @amotl https://github.com/crate/crate-docs-theme/blob/main/src/crate/theme/rtd/crate/static/css/crateio-rtd.css#L27-L31 is indeed the main issue, if we remove those lines, the quote works. But if you have a paragraph wrapped in there, we might also need to override the margin-bottom, else it won't be centered vertically:

image

Should I whip something up? And can you check if we're actually using the current blockquote somewhere in the docs @matkuliak?

Copy link
Member Author

@amotl amotl Jul 18, 2023

Choose a reason for hiding this comment

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

Should I whip something up?

Please go ahead. Thanks!

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you for submitting GH-380, aiming to fix this.

Copy link
Member Author

Choose a reason for hiding this comment

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

@amotl amotl requested a review from msbt July 17, 2023 13:29
@msbt msbt mentioned this pull request Jul 19, 2023
amotl added 6 commits July 20, 2023 00:42
- myst-parser
- sphinx-design
- sphinx-inline-tabs
- sphinx-subfigure
- sphinx-togglebutton
- sphinxcontrib-mermaid
- myst-parser
- sphinx-design
- sphinx-inline-tabs
- sphinx-subfigure
- sphinx-togglebutton
- sphinxcontrib-mermaid
@amotl amotl force-pushed the amo/modernize-nt branch from 648ba49 to 432693b Compare July 19, 2023 22:42
Comment on lines +10 to +14
Todo items
==========
.. todo::

Foo bar baz.
Copy link
Member Author

@amotl amotl Jul 20, 2023

Choose a reason for hiding this comment

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

Apparently, the .. todo directive renders itself with a bit too much of a left padding or margin. Maybe we can do something about it on a future iteration. It is not too important right now.

image

-- https://crate-docs-theme--378.org.readthedocs.build/en/378/rst/misc.html
-- https://crate-docs-theme--378.org.readthedocs.build/en/378/myst/misc.html

Copy link
Member Author

@amotl amotl Jul 20, 2023

Choose a reason for hiding this comment

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

Created GH-381 in order to track this issue.

- The legacy feature gallery exclusively uses reStructuredText.
- The modernized feature gallery demonstrates both reStructuredText and
  Markedly Structured Text syntax.
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.

5 participants