Skip to content

Conversation

@yakov116
Copy link
Member

@yakov116 yakov116 commented Mar 19, 2021

Fixes #4129

Test URL's

This PR

Linkify issues:
mozilla/addons-linter#76

@yakov116 yakov116 added the bug label Mar 19, 2021
@yakov116 yakov116 changed the title Restore rgh-linkify-features in titles Restore rgh-linkify-features and linkify-code in titles Mar 21, 2021
@yakov116 yakov116 marked this pull request as ready for review March 21, 2021 11:59
@yakov116 yakov116 requested a review from fregante March 21, 2021 12:01
@yakov116 yakov116 changed the title Restore rgh-linkify-features and linkify-code in titles Restore rgh-linkify-features and linkify-code in titles Mar 21, 2021
Copy link
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.

Untested but LGTM

@fregante fregante changed the title Restore rgh-linkify-features and linkify-code in titles Restore rgh-linkify-features and linkify-code in titles Mar 23, 2021
@fregante fregante changed the title Restore rgh-linkify-features and linkify-code in titles Restore rgh-linkify-features and linkify-code in titles Mar 23, 2021
@fregante fregante changed the title Restore rgh-linkify-features and linkify-code in titles Restore rgh-linkify-features and linkify-code in titles Mar 23, 2021
@fregante fregante changed the title Restore rgh-linkify-features and linkify-code in titles Restore rgh-linkify-features and linkify-code in titles Mar 23, 2021
@fregante fregante changed the title Restore rgh-linkify-features and linkify-code in titles Restore rgh-linkify-features and linkify-code in titles Mar 23, 2021
@fregante
Copy link
Member

I checked the listener that has been modified in #4089 and it appears to work correctly (i.e. it's run the right number of times)

Also I found which part still uses textContent until it receives the markdown version of the title:

gif

@yakov116
Copy link
Member Author

Also I found which part still uses textContent until it receives the markdown version of the title:

Now I see it, is it an issue? Also we need to anyways wait for GitHub to format it since we need it to in <code/>

@fregante
Copy link
Member

No, it can be merged as is. This is an issue on GitHub's part. We don't deal with titles anymore. Actually it's best to drop that selector from parse-backticks as soon as possible.

@yakov116
Copy link
Member Author

Actually it's best to drop that selector from parse-backticks as soon as possible.

Only .js-issue-title needed to be removed

@yakov116 yakov116 changed the title Restore rgh-linkify-features and linkify-code in titles Restore rgh-linkify-features and linkify-code in titles Mar 23, 2021
@yakov116 yakov116 changed the title Restore rgh-linkify-features and linkify-code in titles Restore rgh-linkify-features and linkify-code in titles Mar 23, 2021
@yakov116 yakov116 merged commit 356b6a6 into main Mar 23, 2021
@yakov116 yakov116 deleted the restore-linkify branch March 23, 2021 17:46
@fregante
Copy link
Member

Are you sure? That selector only matches the conversation title in lists, but conversation titles appear in all sorts of places with different selectors, like the newsfeed, right? We surely have more than one selector

@yakov116
Copy link
Member Author

Are you sure? That selector only matches the conversation title in lists, but conversation titles appear in all sorts of places with different selectors, like the newsfeed, right? We surely have more than one selector

We sure do. I thought you wanted me to drop only things related to the title.

If i have time I will do it tomorrow

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.

The *feature-linkification* _part_ of format-conversation-titles appears to be broken

2 participants