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

All linkifier features broken on new file viewer #6336

Open
fregante opened this issue Feb 11, 2023 · 19 comments
Open

All linkifier features broken on new file viewer #6336

fregante opened this issue Feb 11, 2023 · 19 comments
Labels
bug help wanted Please! ♥︎ Particularly useful features that everyone would love!

Comments

@fregante
Copy link
Member

fregante commented Feb 11, 2023

Description

Screenshot
Screenshot

How to replicate the issue + URL

It's not enabled on
https://github.com/refined-github/refined-github/blob/be756e68dd4bf24d53e802e1fec126506979fa80/test/linkify-urls-in-code.js

But you can see it working in this live code embed:

test('isIssue', batchTestText, linkifyURLsInCode.hasIssue, [
'#1',
'#12',
'an issue in the middle #123 of some text' // Only link to issues in comments, see #381
], [
'#a',
'1234'
]);
test('findURLs', batchTestArray, linkifyURLsInCode.findURLs, [
'http://github.com/',
'https://www.github.com',
'https://github.com/orgs/test/dashboard',
'a long string of text with a url at the end: https://github.com/dashboard?param=test'
], [
'https://github',
'github',
'github.com',
'github/whatever/text'
]);

Extension version

23.2.1

Browser(s) used

Safari

@fregante fregante added the bug label Feb 11, 2023
@fregante fregante changed the title linkify-code broken linkify-code broken on new file viewer Feb 11, 2023
@fregante fregante added help wanted Please! ♥︎ Particularly useful features that everyone would love! labels Feb 11, 2023
@amanmalh
Copy link
Contributor

Hi @fregante, I need more clarification on this. I tested this on chrome, as well as safari. The URLs under 'findURLs' test seem to be working fine for me. The issue reference in the comment is working fine as well. What exactly is broken?

@fregante
Copy link
Member Author

Thank you for looking into this! It's related to the new code view, you'll have to enable it to see the issue: https://docs.github.com/en/search-github/github-code-search/about-github-code-search#enabling-and-disabling-the-new-code-search-and-code-view-beta

@amanmalh

This comment was marked as outdated.

@StefanLobbenmeierObjego
Copy link

Might be nice in general to have refined-github check if preview features are enabled, and if it does not work properly instead have a message that reminds you to try turnin off the preview-features

@fregante
Copy link
Member Author

fregante commented Mar 3, 2023

Easier said than done 😅

Given they just enabled the beta for everyone, I think the opposite should be suggested.

  1. We fix the feature on the new view
  2. User notices it's broken
  3. We suggest switching to the new view

@kidonng
Copy link
Member

kidonng commented Apr 6, 2023

This seems tricky because there's a <textarea> overlay above the actual code. We can linkify the code but no one is able to click them 😅

Or alternatively, we catch the click events and re-dispatch them ourselves 🤔

@fregante
Copy link
Member Author

fregante commented Apr 6, 2023

🤔 what's that textarea for? Is the code inside it? How is the user able to select it?

@kidonng
Copy link
Member

kidonng commented Apr 6, 2023

Presumably for getting cursor position and acting on it, as it is easier than dealing with text nodes.

The real bummer is that the highlighted code is now pseudo-elements:

.react-file-line [data-code-text]::before {
    content: attr(data-code-text);
}

Which means in addition to dealing with clicks, we need to construct the DOM back ourselves. And a11y will suffer.

I can't think of an efficient way to do all these without breaking GitHub functionality.

@fregante

This comment was marked as outdated.

@kidonng
Copy link
Member

kidonng commented Apr 6, 2023

What's the output of $('#read-only-cursor-text-area') in your console?

@fregante
Copy link
Member Author

fregante commented Apr 6, 2023

Always null, even on repos I can't modify and regardless of isPermalink status

Edit: it's there now

@fregante

This comment was marked as outdated.

@kidonng

This comment was marked as outdated.

@fregante
Copy link
Member Author

after

This CSS successfully brings the element back to the surface:

[data-code-text="'https://www.github.com'"] {
    background: red;
    z-index: 10000;
    pointer-events: all
}
[data-code-text="'https://www.github.com'"]:hover {
    background: yellow
}

The issue is that those elements aren't easily editable.

I think CSS anchoring (2024) will be another way to overlay content without altering the DOM, but it's not here yet:

@tonglil
Copy link

tonglil commented Aug 4, 2023

Perhaps this is also the issue with symbolic links not linking any more.

@fregante
Copy link
Member Author

fregante commented Aug 4, 2023

Yes

@fregante fregante changed the title linkify-code broken on new file viewer All linkifier features broken on new file viewer Aug 4, 2023
@134130
Copy link
Contributor

134130 commented Aug 18, 2023

🤔 We can implement in two ways.

  • Just re-using GitHub's highlighting elements.
    • Simple and light way. Not harming GitHub's text editor
  • Overlaying with display: absolute | related
    • Looks more pretty since we can use anchor tag with hovering css.

And this is my rough implement with first way.
화면 기록 2023-08-18 23 46 41

@fregante
Copy link
Member Author

fregante commented Aug 18, 2023

How does the first way work? Can you send a minimal PR? Or demo code?

Edit: I think you're suggesting the demo GIF I used earlier. The problem is that that could only highlight the entire string instead of just the URL. This would not work if a comment has a link and especially if it has more than one link.

The second suggestion would work if we add a single clickable icon somewhere nearby, but we cannot highlight the exact text or potentially even the exact line (if a comment wraps)

@134130
Copy link
Contributor

134130 commented Aug 19, 2023

Exactly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug help wanted Please! ♥︎ Particularly useful features that everyone would love!
Development

No branches or pull requests

6 participants