-
Notifications
You must be signed in to change notification settings - Fork 327
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
center figure and captions inside figwith, while the whole figure is left/right #1750
Conversation
center figure and captions inside figwith, while the whole figure is left/right
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM though see my one note/hesitation about caption alignment.
|
||
p { | ||
text-align: left; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is the only line I have some hesitation about. It looks odd (to my eye anyway) to have L-aligned caption text for an image that is C-aligned within an R-floated box. I think I'd rather default to C-aligning captions. This isn't a super-strong feeling though, no worries if the other maintainers disagree.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
C-aligned for one line text, and L-aligned for muti-lines text, as the same of LaTex.
L-aligned muti-lines is better than C-aligned, especially when the last line is short.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find that jump from L-aligned to C-aligned based on multi/single lines annoying as that means we can have multiple instances of each across a single doc site which makes this look inconsistent.
I cannot wrap my head around how the L-aligned + C-aligned (text and image) thing looks so I will have to look at the preview tomorrow (I am only checking this on mobile).
From an accessibility and readability POV L-aligned is always suggested over C-aligned so if I were to choose one of the other I'd choose L-align always (for left to right text, then R-aligned for right to left languages).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
L-align: for left to right languages, then R-aligned for right to left languages.
Good point. But I don't really know how to realize it. Mainly I don't know how to determine the language type.
Could you please modify it directly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't believe we have robust LR support atm so this might be for another scoped piece of work so for now let's aim for L-aligned.
@gabalafou, this PR has been approved and is waiting for the merge for a bit. I can go ahead an merge but would love a quick check on your end if possible? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It makes me a little bit uncomfortable to change the expected display type of some of these elements, like p
, a
, img
—I think my preference would be to use flexbox to do most of the alignment, so I might submit a follow-up pull request, but in the meantime, I say let's go ahead and merge this in.
…left/right (pydata#1750) * centered figure while left/right center figure and captions inside figwith, while the whole figure is left/right * Update _figures.scss * Update _figures.scss * [pre-commit.ci] Automatic linting and formatting fixes * Update src/pydata_sphinx_theme/assets/styles/content/_figures.scss --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Tania Allard <taniar.allard@gmail.com> Co-authored-by: gabalafou <gabriel@fouasnon.com>
center figure and captions inside figwith, while the whole figure is left/right
testing code: ( MyST md )
old css:
new css: