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

Use logical instead of physical block margins #206

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

emonadeo
Copy link

@emonadeo emonadeo commented Jun 18, 2024

I am currently trying to use Capsize with text, that is rotated 90deg (using writing-mode: vertical-lr;). The current implementation always applies margin-top and margin-bottom, regardless of the actual orientation.

This PR replaces margin-top with margin-block-start and margin-bottom with margin-block-end, as these properties respect aforementioned text orientation.

From MDN:

This property corresponds to the margin-top and margin-bottom, or the margin-right and margin-left properties, depending on the values defined for writing-mode, direction, and text-orientation.

However while this fixes text in vertical writing mode, it does not work with text in vertical writing mode and upright text orientation:

writing-mode: vertical-lr; // or vertical-rl;
text-orientation: upright;

This incorrectly (in the case of capsize) applies left and right margins when using start and end.

I personally think breaking this in favor of supporting vertical text is a worth trade-off, but this is of course up for discussion. Alternatively, this could be an extra opt-in option instead of replacing the current behavior.

I am not sure if this would affect east asian languages that may be written vertically. To my knowledge these are also written horizontally on the web.

@emonadeo emonadeo requested a review from a team as a code owner June 18, 2024 01:12
Copy link

changeset-bot bot commented Jun 18, 2024

🦋 Changeset detected

Latest commit: cfbd642

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@capsizecss/core Major
@capsizecss/vanilla-extract Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@michaeltaranto
Copy link
Contributor

Hey thanks for opening this. We havent been able to use this until very very recently with due to our browser support policy. I wonder if that would be a factor for others too?

Not wanting to add config for no reason, maybe we release this as a breaking change and then if there is a ground swell of people needing the explicit margin direction due to pre Safari 12, then we can add an option then.

How's that sound?

@emonadeo
Copy link
Author

Sounds good to me. I am down to implement that option if that comes to be the case. Feel free to tag me then, as I probably won't be actively monitoring new issues of this repo.

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