feat: adding gh changelog/releases to npmx#1233
feat: adding gh changelog/releases to npmx#1233WilcoSp wants to merge 151 commits intonpmx-dev:mainfrom
Conversation
…md -> html does need to change)
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Lunaria Status Overview🌕 This pull request will trigger status changes. Learn moreBy default, every PR changing files present in the Lunaria configuration's You can change this by adding one of the keywords present in the Tracked Files
Warnings reference
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
excludes changelog releases due to needing api calls
This is what I was wondering about in previous comments. Maybe we shouldn't support files, but only display what's in releases. In the case of files, an additional question is whether we should load a file from a specific version (and which hash is that?). And another big question is, maybe we should only show the changelog for the current version (incl. show tab only if there is block for this specific version)... I left another small comment. About other details, I think we can continue to improve iteratively - it seems we also could refactor the Markdown renderer and reuse it more with the main page In general lgtm, thanks 🤍 |
I don't think it's a good idea to only support the releases. by removing changelog.md file support it'll make this changelog feature incomplete.
This will make serving the changelog overcomplicated and might result in a worse experience.
For a specific version @ShroXd is currently working on making it possible to have changelogs at the versions's page Also maybe this will helps for the context of how I've implemented the changelogs for npmx.
For markdown rendering refactor I want to save for a separate smaller pr that's easier to review & merge (tbh I also don't want to deal with bigger merge conflicts rn)
That was always the plan, at #501 there is a roadmap for what is planned. |
It's just a few more specific cases. And as the result bunch of new questions:
|
yes, atm we're completely relying on ungh.cc & their use of the github api. using github api directly isn't possible because of their stricter rate limits. so atm idk a good solution with also the fact there are plans to support other git providers
at least in this pr there is no size limit. a solution could be to split a changelog.md file up into chunks. but idk if that is a good solution. giving "changelog.md is too big" isn't a very good option because that could be annoying.
I do think it's better to still give the changelogs instead of not giving them, even if the specific version doesn't have them. sometimes no information is still useful information
for this we currently don't have anyway to support when the changelog is on a alpha/beta branch because everyone does this differently. maybe the
idk whether it's possible but in case ungh.cc is unable to return any info when trying to detect releases/changelog.md we could if possible look up in the case for a previous versions and server that. |
|
A few nitpicks:
Other than that, nice work! Can’t wait to see how the feature set will turn out when all the features are done! |
gameroman
left a comment
There was a problem hiding this comment.
One thing I am not a fan of is autoscroll when switching from main tab to changelog tab
Looks like it's triggered due to automatic addition of hash to URL
|
Otherwise this looks really great, I like it |
I had added it to bring to the requested version or at least as close as possible.
ehm, skeletons loaders are already being used with
In the package readme they also have it that every header is anchored, that is why it's also with changelogs.
For small releases it's not needed, but for very long ones (nuxt/tsdown) it's still handy to have and the toc is always generated no matter how small, there is only a requirement that at least 2 toc items are needed to show the toc atm.
I'll add a atm I've also work so either after work or tomorrow I'll make the changes. |
fixed hash being applied when navigating to main added setting to en-/disable auto scrolling updated focus styling in packageHeader header anchor is now by default not visible changelog release cards are lazy & hydrate on interaction using useLazyFetch to prevent blocking
|
@WarningImHack3r & @gameroman I've just committed the changes that I talked about in my prev msg. I've also fixed that hash was being applied to the url when switching to main & the cards for releases are now lazy with "hydrate-on-interaction", this makes initial rendering faster. |
WarningImHack3r
left a comment
There was a problem hiding this comment.
Another set of QA feedback (:D):
- The anchor icon that's now on hover for releases' body is only visible when hovering on it, which can be surprising when you don't expect it. I think it should be visible when hovering over the whole title it's anchored to.
- Tag navigation is a bit buggy
- Some selections are misplaced (here, link from above in the PR)
- Some others do nothing (navigating from the latest (default) Vite to any other version requires a refresh to work)
- Some others crash (try navigating here to
0.21.0-beta.5with the dropdown), and pressing the back button breaks the X-axis layout
On another note, here are the promised French localizations:
{
// ...
"enable_changelog_autoscroll": "Scroller automatiquement à la version voulue",
"enable_changelog_autoscroll_description": "Scroller automatiquement à — ou près — de la version voulue dans la page des changements"
// ...
"changelog": "changements"
// ...
"changelog": {
"pre_release": "Pré-version",
"draft": "Brouillon",
"no_logs": "Désolé, ce paquet ne publie pas de changements ou le format de ses changements n'est pas pris en charge.",
"error": {
"p1": "Désolé, les changements de {package} n'ont pas pu être chargés",
"p2": "Veuillez réessayer plus tard ou {viewon}" // i couldn't find what `viewon` is
},
"rate_limit_ungh": "Désolé, la limite de requêtes à l'API GitHub a été atteinte, réessayez plus tard"
}
}…e whole header to make it easier to find
…ns, hopefully it resolves some issues
|
I've committed some changes that should improve changelogs.
@WarningImHack3r the issues you had found should have be fixed now. |
WarningImHack3r
left a comment
There was a problem hiding this comment.
Awesome work!!
Here is the translated new entry:
"version_unavailable": "La version demandée n'est pas disponible."It's the only one that changed as far as I can see.
There is also a moved block in the french translation, idk if it's intentional
LGTM otherwise!
This pr will add the possibility to view the changelog.md & releases from a package's github repo within npmx.
This will make it easier to see the changelogs while not needing to leave npmx and allowing quicker access.
This pr is the first pr of #501
Preview here, this will automatically update
Acknowledgements
While I was making this PR antfu also made this pr #1368 and here my comment from his pr
Also @ShroXd is currently working on adding changelogs to the version history page that was merged with #2025, we've both agreed to first independently work on our PRs and later after both are merged and we've received feedback & irl use we'll collaborate on the interaction between the two
Translations
Thanks @WarningImHack3r for translating the text for changelog to French