`linkify-line-numbers` - Fix occasional layout break by fregante · Pull Request #9531 · refined-github/refined-github · GitHub
Skip to content

linkify-line-numbers - Fix occasional layout break#9531

Closed
fregante wants to merge 4 commits into
mainfrom
linkify-no-replace
Closed

linkify-line-numbers - Fix occasional layout break#9531
fregante wants to merge 4 commits into
mainfrom
linkify-no-replace

Conversation

@fregante

@fregante fregante commented May 19, 2026

Copy link
Copy Markdown
Member

@github-actions github-actions Bot added the bug label May 19, 2026
fregante
fregante commented May 19, 2026
View reviewed changes
Comment thread source/features/linkify-line-numbers.tsx

@fregante fregante May 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Here's my guess of what happened

  1. we replace the td with an a (already not great)
  2. GitHub defer-loads the diff somehow
  3. they attempt to replace the td, don't find it, so the numbers are gone
  4. the rest of the DOM is recreated around all the new a elements, which somehow float to the top:
Image

@SunsetTechuila

SunsetTechuila commented May 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

It makes more sense to revert the changes made in #9354. Why: 3f41085 (#9354) (description)

@SunsetTechuila

This comment was marked as resolved.

Sign in to view
@fregante

fregante commented May 19, 2026

Copy link
Copy Markdown
Member Author

Hotfix:

This won't restore the line numbers. The hotfix is to disable the feature

@fregante

fregante commented May 19, 2026

Copy link
Copy Markdown
Member Author

It makes more sense to revert the changes made in #9354

Funny thing is that I almost commented that we should use delegate here, forgetting that it was that way just last month.

Could go either way, really. The reduced hit area isn't a big deal once the native td {cursor:pointer} is overridden.

@fregante fregante marked this pull request as ready for review May 19, 2026 16:24
fregante added a commit to refined-github/yolo that referenced this pull request May 19, 2026
@fregante
linkify-line-numbers hotfix disable
417a5ab 
refined-github/refined-github#9511
refined-github/refined-github#9531
@fregante

fregante commented May 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

revert the changes made in #9354

Would you then just use onAlteredClick instead of delegate to add support for middle click? Can you open a PR replacing this one if you want to go that way?

@SunsetTechuila SunsetTechuila mentioned this pull request May 19, 2026
Merged
SunsetTechuila
SunsetTechuila reviewed May 19, 2026
View reviewed changes
Comment thread source/features/linkify-line-numbers.tsx

@SunsetTechuila SunsetTechuila May 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We can't use Link--primary or Link--secondary here because the text color can be either secondary or primary

@SunsetTechuila SunsetTechuila deleted the linkify-no-replace branch May 21, 2026 10:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@SunsetTechuila SunsetTechuila SunsetTechuila left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

bug

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

linkify-line-numbers moving the number to the top

2 participants

@fregante @SunsetTechuila