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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add jump-to-change-requested-comment feature #3710

Merged
merged 9 commits into from
Nov 12, 2020
Merged

Add jump-to-change-requested-comment feature #3710

merged 9 commits into from
Nov 12, 2020

Conversation

xtianatilano
Copy link
Contributor

@xtianatilano xtianatilano commented Nov 9, 2020

Thanks for contributing! 馃崉

  1. LINKED ISSUES:
    Closes #2712

  2. TEST URLS:
    #17968

  3. SCREENSHOT:

link-to-requested-change-comment

@fregante
Copy link
Member

fregante commented Nov 9, 2020

Thanks for the PR! That area of the page is updated via ajax, so it needs to use selector-observer. Search this in other features to see how it鈥檚 used

@xtianatilano
Copy link
Contributor Author

I updated the feature to use selector-observer, let me know if it is correct.

readme.md Outdated Show resolved Hide resolved
source/features/link-to-requested-change-comment.tsx Outdated Show resolved Hide resolved
Comment on lines 16 to 18
if (element) {
wrap(element, <a href={messageContainer.href}/>);
}
Copy link
Member

Choose a reason for hiding this comment

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

Conditions like this makes any future errors silent, however this element is expected to be "not found" when GitHub starts hiding some comments on long discussions.

It's not ideal, but we don't have a reasonable solution here.

@fregante fregante changed the title add link-to-requested-change-comment feature Add link-to-requested-change-comment feature Nov 10, 2020
readme.md Outdated Show resolved Hide resolved
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.

馃コ

@xtianatilano xtianatilano changed the title Add link-to-requested-change-comment feature Add jump-to-change-requested-comment feature Nov 12, 2020
@fregante fregante changed the title Add jump-to-change-requested-comment feature Add jump-to-change-requested-comment feature Nov 12, 2020
@fregante fregante merged commit e158e03 into refined-github:master Nov 12, 2020
@xtianatilano
Copy link
Contributor Author

馃コ

Thank you! 馃コ

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

Link to review from the overview at the end of a PR
3 participants