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

Consider rolling back the global word-break: break-word style #1402

Closed
3 tasks done
fvsch opened this issue Sep 26, 2022 · 4 comments · Fixed by #1405
Closed
3 tasks done

Consider rolling back the global word-break: break-word style #1402

fvsch opened this issue Sep 26, 2022 · 4 comments · Fixed by #1405
Labels
bug Something isn't working theme Related to the theme

Comments

@fvsch
Copy link

fvsch commented Sep 26, 2022

Describe the bug

A recent update (#1064) added a word-break: break-word style to all HTML elements. This leads to unpleasant results such as this in table cells (see the first and last column):

Table after

While before this update the result was much more readable:

Table before

Reproduction

StackBlitz reproduction with a table:
https://stackblitz.com/edit/vitepress-theme-word-break-issue?file=index.md

Expected behavior

Risky styles such as word-break: break-word should not be applied to all elements.

Handling overflowing content (if any) should be done at the component level, probably with specific solutions for each component.

System Info

System:
    OS: macOS 12.6
    CPU: (8) arm64 Apple M1
    Memory: 117.00 MB / 8.00 GB
    Shell: 3.5.1 - /opt/homebrew/bin/fish
  Binaries:
    Node: 16.17.1 - /opt/homebrew/opt/node@16/bin/node
    Yarn: 1.22.19 - /opt/homebrew/bin/yarn
    npm: 8.19.1 - /Volumes/Pieces/.npm/bin/npm
  Browsers:
    Chrome: 105.0.5195.125
    Firefox Nightly: 107.0a1
    Safari: 16.0
  npmPackages:
    vitepress: ^1.0.0-alpha.16 => 1.0.0-alpha.16

Additional context

No response

Validations

  • Follow our Code of Conduct
  • Read the docs.
  • Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
@fvsch fvsch added the bug: pending triage Maybe a bug, waiting for confirmation label Sep 26, 2022
@brc-dd
Copy link
Member

brc-dd commented Sep 26, 2022

Ah right. What if we set overflow-wrap: break-word; instead?

@brc-dd brc-dd added bug Something isn't working theme Related to the theme and removed bug: pending triage Maybe a bug, waiting for confirmation labels Sep 26, 2022
@fvsch
Copy link
Author

fvsch commented Sep 26, 2022

It looks like overflow-wrap: break-word would have a less drastic result, while still fixing the issue for e.g. long continuous strings in content. So that could be a good fix.

Personally, I'd still be in favor of not applying styles to all elements. The * { box-sizing: border-box } technique has been battle-tested for 15 years, but defining other styles on all elements tends to have unintended consequences in my experience.

Maybe a more targeted style for markdown content?

:where(.vp-doc) :is(h1, h2, h3, h4, h5, h6, p, li) {
  overflow-wrap: break-word;
}

or, if applying to table cells make sense (not sure it does for us, but I’d have to check against a bunch of content), maybe target td elements as well:

:where(.vp-doc) :is(h1, h2, h3, h4, h5, h6, p, li, td) {
  overflow-wrap: break-word;
}

@brc-dd
Copy link
Member

brc-dd commented Sep 26, 2022

Actually stuff not inside .vp-doc was overflowing too (like in #1088). We probably can just use that on all h* and p elements, irrespective of vp-doc/vp-home?

@brc-dd
Copy link
Member

brc-dd commented Sep 27, 2022

I've reverted #1064. Haven't added anything new, we will discuss later what might be the best option here.

@brc-dd brc-dd closed this as completed Sep 27, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working theme Related to the theme
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants