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.

Sign up for GitHub

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

Always break long branch-names #2002

Merged
merged 5 commits into from
Apr 3, 2022
Merged

Always break long branch-names #2002

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e1a389e
Break long branch names
simurai Mar 29, 2022
82cbde8
Create early-spoons-greet.md
simurai Mar 29, 2022
25ba39a
Stylelint auto-fixes
actions-user Mar 29, 2022
f6997d0
Merge branch 'main' into fix-branch-name
simurai Apr 1, 2022
211a27e
Merge branch 'main' into fix-branch-name
simurai Apr 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/early-spoons-greet.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/css": patch
---

Always break long `branch-name`s
1 change: 1 addition & 0 deletions src/branch-name/branch-name.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
padding: 2px 6px;
font: 12px $mono-font;
color: var(--color-fg-muted);
word-break: break-all;
Copy link
Contributor

Choose a reason for hiding this comment

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

@simurai I did a little reading on word-break (did you know break-word is deprecated? 😮) and I'm wondering if this would solve for the - dash scenario you mentioned in the PR description:

Suggested change
word-break: break-all;
overflow-wrap: anywhere;
word-break: normal;

Copy link
Contributor Author

@simurai simurai Mar 30, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

@langermank overflow-wrap: anywhere; still seems not supported in Safari.

Another thing with dashes/hyphens is that if the branch name starts with a dash/hyphen and follows with dots, underlines, slashes... it prioritizes that first dash/hyphen, leaving a big gap after. So maybe for now and this specific case with branch names, using word-break: break-all might still be the better option? Even tough it's deprecated. 😅

overflow-wrap: anywhere; word-break: break-all;
Screen Shot 2022-03-30 at 13 19 23 Screen Shot 2022-03-30 at 13 20 05

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh shoot! I didn't check the browser support. I would just keep break-all then. It's not deprecated, break-word is (sorry, didn't mean to confuse you). I was just surprised by that as I was thinking of recommending it!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, cool. In that case, mind approving? 🙇

background-color: var(--color-accent-subtle);
border-radius: $border-radius;

Expand Down