Skip to content

linkify-code, show-whitespace - Restore on React-based code views and diffs#8878

Merged
fregante merged 15 commits into
refined-github:mainfrom
SunsetTechuila:linkify
Feb 3, 2026
Merged

linkify-code, show-whitespace - Restore on React-based code views and diffs#8878
fregante merged 15 commits into
refined-github:mainfrom
SunsetTechuila:linkify

Conversation

@SunsetTechuila
Copy link
Copy Markdown
Member

@SunsetTechuila SunsetTechuila commented Jan 18, 2026

@github-actions github-actions Bot added the bug label Jan 18, 2026
@SunsetTechuila SunsetTechuila changed the title Restore linkify features on React-based views linkify-code - Restore on React-based code views and diffs Jan 19, 2026
@SunsetTechuila

This comment was marked as outdated.

@SunsetTechuila SunsetTechuila marked this pull request as ready for review January 19, 2026 03:07
@SunsetTechuila SunsetTechuila marked this pull request as draft January 19, 2026 03:52
@SunsetTechuila SunsetTechuila marked this pull request as ready for review January 19, 2026 09:50
Comment thread source/manifest.json Outdated
@SunsetTechuila

This comment was marked as resolved.

@aaemnnosttv
Copy link
Copy Markdown

Does this PR also fix show-whitespace as shown in the screenshot?
#8504 (comment)

@SunsetTechuila

This comment was marked as outdated.

@SunsetTechuila
Copy link
Copy Markdown
Member Author

It seems like it does actually. I'm confused

':is(.snippet-clipboard-content, .highlight) > pre.notranslate', // Code blocks in comments. May be wrapped twice
'.comment-body code:not(a code, pre code)', // Inline code in comments
'.diff-text-inner',
'.react-code-text:not(.react-line-number)',
Copy link
Copy Markdown
Member Author

@SunsetTechuila SunsetTechuila Jan 26, 2026

Choose a reason for hiding this comment

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

#8878 (comment)

That's because I've updated codeElementsSelector

This comment was marked as outdated.

@fregante
Copy link
Copy Markdown
Member

fregante commented Jan 28, 2026

This is really cool, it works. One thing I noticed is that it breaks horizontal mousewheel scrolling when the cursor is on one of the anchors. This is most likely due to the DOM location of the links

gif

This is a relatively minor annoyance, but perhaps it has an easy fix.

Use the feature itself as a test URL:

@fregante
Copy link
Copy Markdown
Member

fregante commented Jan 28, 2026

Also I'm seeing the links appear and then disappear as the UI settles: 5e1cb0e

Demo, a page reload:

gif

I'd handle this in a separate PR or just open a new tracking issue. Fixing some views is better than pushing this PR further. We can ship it by v26.2.1

Copy link
Copy Markdown
Member

@fregante fregante left a comment

Choose a reason for hiding this comment

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

Can be merged as is and then followed by 2 tracking issues or 1/2 PRs

@fregante
Copy link
Copy Markdown
Member

@SunsetTechuila if this PR doesn't fix all features mentioned by #6336, it should be unlinked or rather a new, more specific followup issue could be opened.

@SunsetTechuila
Copy link
Copy Markdown
Member Author

SunsetTechuila commented Jan 28, 2026

Is anything other than linkify-code mentioned in #6336? Both issue refs and URLs are processed by that one feature

@fregante
Copy link
Copy Markdown
Member

I was under the impression that our many "linkify" features were also affected. If not, no problem

@SunsetTechuila
Copy link
Copy Markdown
Member Author

SunsetTechuila commented Jan 28, 2026

There is one more feature broken: linkify-symbolic-links

@SunsetTechuila
Copy link
Copy Markdown
Member Author

SunsetTechuila commented Jan 28, 2026

This is a relatively minor annoyance, but perhaps it has an easy fix.

Can't come up with anything other than

clonedLink.addEventListener("wheel", (event) => {
  const scrollValue = event.shiftKey ? event.deltaY : event.deltaX;
  textarea.scrollLeft += scrollValue;
  event.preventDefault();
}, { passive: false });

And this isn't good. I wish you could just re-dispatch the event on a different element

@fregante
Copy link
Copy Markdown
Member

How is GitHub dealing with it? If you move the anchors inside the element that has the horizontal overflow, it should work.

@SunsetTechuila
Copy link
Copy Markdown
Member Author

SunsetTechuila commented Jan 28, 2026

If you move the anchors inside the element that has the horizontal overflow

The textarea and elements under it have horizontal overflow

@fregante fregante merged commit 193b603 into refined-github:main Feb 3, 2026
8 checks passed
@SunsetTechuila
Copy link
Copy Markdown
Member Author

SunsetTechuila commented Feb 3, 2026

This is a relatively minor annoyance, but perhaps it has an easy fix.

Can't come up with anything other than

clonedLink.addEventListener("wheel", (event) => {
  const scrollValue = event.shiftKey ? event.deltaY : event.deltaX;
  textarea.scrollLeft += scrollValue;
  event.preventDefault();
}, { passive: false });

And this isn't good. I wish you could just re-dispatch the event on a different element

@fregante so, is this better than nothing or not?

@fregante
Copy link
Copy Markdown
Member

fregante commented Feb 4, 2026

It's worth a try. What kind of mouse/trackpack do you have?

@SunsetTechuila
Copy link
Copy Markdown
Member Author

SunsetTechuila commented Feb 4, 2026

A mouse without the ability to scroll horizontally, if that's what you're asking

@fregante
Copy link
Copy Markdown
Member

fregante commented Feb 4, 2026

If you are on Windows it looks like you can scroll horizontally by holding the shift key down. You can try it out and send a PR, I'll try it on my trackpad later.

My main concern was that most mice scroll in large jumps while trackpads move smoothly. A lot of scroll-related JS code doesn't understand this difference and will make trackpad scrolls unusable.

@SunsetTechuila
Copy link
Copy Markdown
Member Author

SunsetTechuila commented Feb 4, 2026

you can scroll horizontally by holding the shift key down.

I know, I've tested that snippet

@SunsetTechuila
Copy link
Copy Markdown
Member Author

SunsetTechuila commented Feb 11, 2026

Also I'm seeing the links appear and then disappear as the UI settles: 5e1cb0e

I don't have this issue anymore.

Links also used to disappeared on hover as spans were re-rendering.

@SunsetTechuila SunsetTechuila changed the title linkify-code - Restore on React-based code views and diffs linkify-code, show-whitespace - Restore on React-based code views and diffs Mar 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

show-whitespace missing from file viewer ⚠️ All linkifier features broken on new file viewer

4 participants