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

Jump to bottom

Refactor tab-size to reduce conflicts #6480

Merged
merged 10 commits into from
Apr 10, 2023
Merged

Refactor tab-size to reduce conflicts #6480

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
e1946da
Set tab size less aggressively
kidonng Apr 9, 2023
e507033
FIx links
kidonng Apr 9, 2023
a91afbc
Move `--tab-size` to `show-whitespaces`
kidonng Apr 9, 2023
457894d
Move default `--tab-size` back
kidonng Apr 9, 2023
7ff44c7
Apply suggestions from code review
kidonng Apr 9, 2023
82e5df3
Resolve previous review at #4833
kidonng Apr 9, 2023
eb6f729
Don't set `--tab-size` on file blocks
kidonng Apr 9, 2023
16c0137
Document; update urls; add missing attrs; always match attr to var
fregante Apr 10, 2023
e69312b
Merge remote-tracking branch 'origin/main' into tab-size
fregante Apr 10, 2023
0e32bbd
Comments
fregante Apr 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
57 changes: 57 additions & 0 deletions source/features/show-whitespace.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,60 @@
background-image: url('data:image/svg+xml,%3Csvg preserveAspectRatio="xMinYMid meet" viewBox="0 0 12 24" xmlns="http://www.w3.org/2000/svg"%3E%3Cpath d="M4.5 11C4.5 10.1716 5.17157 9.5 6 9.5C6.82843 9.5 7.5 10.1716 7.5 11C7.5 11.8284 6.82843 12.5 6 12.5C5.17157 12.5 4.5 11.8284 4.5 11Z" fill="rgba(128, 128, 128, 50%25)"/%3E%3C/svg%3E');
background-size: 1ch 1.25em;
}

[data-tab-size='1'] {
--tab-size: 1;
}
kidonng marked this conversation as resolved.
Show resolved Hide resolved

[data-tab-size='2'] {
--tab-size: 2;
}

[data-tab-size='3'] {
--tab-size: 3;
}

[data-tab-size='4'] {
--tab-size: 4;
}

[data-tab-size='5'] {
--tab-size: 5;
}

[data-tab-size='6'] {
--tab-size: 6;
}

[data-tab-size='7'] {
--tab-size: 7;
}
Comment on lines +44 to +46
Copy link
Member Author

@kidonng kidonng Apr 10, 2023 •
edited

Choose a reason for hiding this comment

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

Does this appear anywhere? Preferences only have 9 options: 1-6, 8, 10, 12. Not all 12 steps are available.

Copy link
Member

Choose a reason for hiding this comment

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

You're right, this was more like (unnecessary) future-proofing 😂

Screenshot

Copy link
Member Author

Choose a reason for hiding this comment

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

I will add this to lint (to-do)


[data-tab-size='8'] {
--tab-size: 8;
kidonng marked this conversation as resolved.
Show resolved Hide resolved
}

[data-tab-size='9'] {
--tab-size: 9;
}

[data-tab-size='10'] {
--tab-size: 10;
}

[data-tab-size='11'] {
--tab-size: 11;
}

[data-tab-size='12'] {
--tab-size: 12;
}

/*
Can't use this because attr() returns strings and casting isn't supported anywhere yet
https://developer.mozilla.org/en-US/docs/Web/CSS/attr

[data-tab-size] {
--tab-size: attr(data-tab-size);
}
*/
47 changes: 18 additions & 29 deletions source/features/tab-size.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,41 +1,30 @@
/* Make tab-indented code more readable on GitHub and Gist */
/*
GitHub doesn’t apply the user’s "tab size" setting in a few places, leaving the 8-character default.
This feature reduces it to 4 characters, while allowing GitHub’s own tab size setting to override it in its children.

* {
tab-size: var(--tab-size) !important;
}
There’s also one place where GitHub incorrectly uses the "user configuration" attribute to set it to 8, so we’re explicitly overriding that. #4833

[data-tab-size='2'] {
--tab-size: 2;
}
--tab-size is used to allow the user to override it and to be picked up by `show-whitespace`
*/

:root, /* Changes the browser default */
.comment-body [data-tab-size='8'], /* User setting is ignored in comments #4833 */
[data-tab-size='4'] {
:root,
.tab-size[data-tab-size='8'] {
--tab-size: 4;
tab-size: var(--tab-size);
}

Comment on lines +10 to 14
Copy link
Member

Choose a reason for hiding this comment

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

The tab-size feature overrides the tab size. This one single selector overrides it. I think this is the most consistent situation we got so far

Copy link
Member Author

@kidonng kidonng Apr 10, 2023 •
edited

Choose a reason for hiding this comment

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

Sorry if my changes have confused you 😢

Could have just reverted eb6f729 (#6480) if you feel the former version is better

Now this selector isn't specific enough (lacking .comment-body) 🤔

I opened a new pull request to fix this

Copy link
Member

Choose a reason for hiding this comment

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

Heh you're right I didn't even notice that I just undid your commit 😅

Copy link
Member Author

Choose a reason for hiding this comment

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

It's fine, I guess I'm just bad at expressing my intent (and tend to complicate things) 😂

[data-tab-size='8'] {
--tab-size: 8;
}
/*

[data-tab-size='10'] {
--tab-size: 10;
}
## Test URLs

[data-tab-size='12'] {
--tab-size: 12;
}
- Rendered documents: https://github.com/refined-github/refined-github/blob/154c0f8c86b23e52d1e1ee947b90bc36bc6188ba/contributing.md#featuresadd
- Code blocks and embeds in conversations: https://github.com/refined-github/sandbox/pull/19
- The comment field everywhere

/*
Can't use this because attr() returns strings and casting isn't supported anywhere yet
https://developer.mozilla.org/en-US/docs/Web/CSS/attr
### Unaffected URLs

[data-tab-size] {
--tab-size: attr(data-tab-size);
}
*/
- isFile: https://github.com/refined-github/refined-github/blob/6979e88e6a7b3273338a2651ac4b533609ef97df/source/options.html
- isEditingFile: https://github.com/refined-github/refined-github/edit/main/source/options.html
- isGist: https://gist.github.com/kidonng/3ab6b5779e1a5a46984c14eecc0fc3a0

/*
Test URLs
https://github.com/refined-github/sandbox/pull/19
*/