react-native-community / upgrade-helper

⚛️ A web tool to support React Native developers in upgrading their apps.
https://react-native-community.github.io/upgrade-helper
MIT License
3.59k stars 109 forks source link

feat: Add support for anchor links #349

Closed blakef closed 1 year ago

blakef commented 1 year ago

Summary

This lets users copy and paste links directly to specific changes in the diff. This is pretty useful for sharing via chat. In the contributors discord I saw a link posted which would have benefited from this.

Screenshots

Anchor link

CleanShot 2023-03-06 at 13 27 57@2x

Anchor link clicked

CleanShot 2023-03-06 at 13 28 08@2x

Loading from link

https://user-images.githubusercontent.com/49578/223138105-f67c9b30-16a2-41ad-94ad-eb1e818beb0a.mp4

Test Plan

Ran unit test and on my localhost.

What are the steps to reproduce?

Checklist

lucasbento commented 1 year ago

@blakef awesome contribution 🥳

I found one small issue, when checking a wider-range diff, it doesn't focus on the correct file, an example can be seen here which should go to the Android.mk file upgrading from 0.69.7 to 0.71.3.

blakef commented 1 year ago

@lucasbento looks like the animation hadn't completed. Fixed now.

https://user-images.githubusercontent.com/49578/223165705-5ef8008d-2914-4eef-8805-3b0864d403ed.mp4

lucasbento commented 1 year ago

@blakef I noticed that if you go to to an URL with a file (this one, for example) and change the version at the top, it immediately goes to the file again, is that intentional? I believe it would be better to stop triggering the scroll on URL changes after the file has been scrolled to.

blakef commented 1 year ago

Thanks for the patience and work you've put into reviewing. Hopefully the final set of changes:

  1. Do we have a fragment in the url → jump once the page has loaded and the animation completes. Don't jump again as long as the page doesn't refresh.
  2. If the user changes the versions (after loading) → check if there is a fragment and remove it.
    • To me this seemed reasonable, because at this point the anchor could be invalid.
    • More importantly the user doesn't get value out of the updated url with the old fragment. They gone from been shown an interesting file on a diff, onto something else. If they subsequently see something worth sharing, the anchor clipboard button should be on-screen.

https://user-images.githubusercontent.com/49578/223575320-82f0f52b-81ab-430c-be64-1c18cd8c78d9.mp4

lucasbento commented 1 year ago

Awesome contribution, thank you for working on this, @blakef!