feat: introduced changelog modal on downloads
Description
This PR introduces the support of ChangelogModals within the Download's page of the redesigned Node.js Website
Note that this PR might increase the client bundle size ð€ we should probably check how much of an increase it is.
Validation
The "changelog button" should open a changelog with the correctly rendered MDX data.
The latest updates on your projects. Learn more about Vercel for Git âïž
| Name | Status | Preview | Comments | Updated (UTC) |
|---|---|---|---|---|
| nodejs-org | â Ready (Inspect) | Visit Preview | ð¬ Add feedback | Mar 25, 2024 7:20pm |
Lighthouse Results
| URL | Performance | Accessibility | Best Practices | SEO | Report |
|---|---|---|---|---|---|
| /en | ð 78 | ð¢ 90 | ð¢ 100 | ð¢ 91 | ð |
| /en/about | ð¢ 100 | ð¢ 91 | ð¢ 100 | ð¢ 91 | ð |
| /en/about/previous-releases | ð¢ 99 | ð¢ 90 | ð¢ 100 | ð¢ 92 | ð |
| /en/download | ð¢ 99 | ð 89 | ð¢ 100 | ð 82 | ð |
| /en/blog | ð¢ 100 | ð¢ 90 | ð¢ 100 | ð¢ 92 | ð |
Note that this version of the ChangelogModal does not include the avatars of all contributors that made a release happen. We still need to hook this into some internal utility for getting the names of all release authors.
I remember @richardlau or @targos mentioning this, but I might be remembering this incorrectly ð€
Unit Test Coverage Report
| Lines | Statements | Branches | Functions |
|---|---|---|---|
| 79.29% (452/570) | 76.19% (144/189) | 69.91% (79/113) |
Unit Test Report
| Tests | Skipped | Failures | Errors | Time |
|---|---|---|---|---|
| 90 | 0 :zzz: | 0 :x: | 0 :fire: | 4.652s :stopwatch: |
I compared the page source sizes for the two versions:
With changelog modal;
Source size: 193.85 KB
Beta without changelog modal;
Source size: 86.65 KB
In addition, when I check with @next/bundle-analyzer, the gzipped dimensions are as follows;
Server feat/changelog-modal;
Server main;
Due to the nature of our page structure, it is not clear exactly what these parts affect. There seems to be an increase only on ../app/[locale]/[[...path]]/page.js.
Client feat/changelog-modal;
Client main;
I'm not sure exactly how we can reduce the size here, but I'll check it out as well (if it's possible) ð
I compared the page source sizes for the two versions:
With changelog modal; Source size:
193.85 KBBeta without changelog modal; Source size:
86.65 KBIn addition, when I check with
@next/bundle-analyzer, the gzipped dimensions are as follows;Server
feat/changelog-modal;Server `main`;
Due to the nature of our page structure, it is not clear exactly what these parts affect. There seems to be an increase only on `../app/[locale]/[[...path]]/page.js`.
Client
feat/changelog-modal;Client `main`;
I'm not sure exactly how we can reduce the size here, but I'll check it out as well (if it's possible) ð
If you can investigate that'd be great! It is indeed a big bundle increase, mostly (I assume) because it is also bundling the Shiki WASM and Languages and Theme on the client-side ð€
Can you check what's the bundle difference if we removed Shiki from the ChangelogModal?
Something feels weird with how the page seems to disappear entirely with no UI element to close the modal.
Something feels weird with how the page seems to disappear entirely with no UI element to close the modal.
Indeed, I assume we need to add an X button. Hmm. @canerakdas as I mentioned before, feel free to hijack this PR :)
In relation to what @targos said, I think we could make the background around the modal more transparent to see website layout.
how will we resolve differences or deltas from the figma's vision and reality of implementation?
I think we should open a discussion/issue to find a way forward with the Figma and potential new features that won't be on this design. And also to know how to adapt the design to the real condition.
my opinion is that feature is not worth the code, bundle size, or UX experiences. why not link to the GitHub release?
Stepping back a bit, what this is doing is trying to present the notes/lists of changes for the release to allow users of the site a way of finding out what changed or is notable about a particular release. In the redesign this is more or less covered by linking through to the blog post for the release. The blog posts are generated via scripts/release-post based on content from the changelog over in the core nodejs/node repository.
If we look at the pre-redesign (i.e. current) website:
- https://nodejs.org/en/download neither links to the changelog nor the blog post.
- The download buttons on the homepage have links underneath to the changelogs on GitHub. These have been problematic because for some releases the file linked to is too large for GitHub to render in the web UI (see for example https://github.com/nodejs/nodejs.org/issues/4816 -- there was also a suggestion there to link to the GitHub release but that's also had size limitations).
- https://nodejs.org/en/about/previous-releases also links to the changelogs on GitHub.
- None of the pages AFAICT link to the blog posts.
So the redesign is already doing something the pre-redesign site did not -- i.e. it presents the changelog information by linking to the blog posts. I think we should keep linking to the blog posts in the redesign and do not need to repeat the information by linking to the changelogs.
Right, the intent of this PR was to solve the issue we have of changelog rendering on GitHub.
But it would only cover/handle cases for the current LTS and current releases, afaik.
Since these changelogs use the same content as from the blog posts, probably it is good enough to just have the link to the release post.
One of the other goals of this Modal, was to have a collection of GitHub avatars rendering, that showcase all the committers that made said release possible, and pretty much highlight their contributions.
I think we could still add this, but directly to the Download page, maybe below the header?
Wdyt @nodejs/nodejs-website
Regarding lack of designers, well, at this point since we do not have anyone here volunteering that works in that field, we lack the experts to assess such cases.
(Also @richardlau I pinged you here because I recall you mentioning there was a way to get a list of all committers of a release, can you point me to the right direction?)
(Also @richardlau I pinged you here because I recall you mentioning there was a way to get a list of all committers of a release, can you point me to the right direction?)
ð I don't recall that.
One of the other goals of this Modal, was to have a collection of GitHub avatars rendering, that showcase all the committers that made said release possible, and pretty much highlight their contributions.
I think we could still add this, but directly to the Download page, maybe below the header?
@ovflowd I didn't really get your idea, you want to have an AvatarGroup on BlogHeader with list of contributor ? and set aside the modal?
One of the other goals of this Modal, was to have a collection of GitHub avatars rendering, that showcase all the committers that made said release possible, and pretty much highlight their contributions. I think we could still add this, but directly to the Download page, maybe below the header?
@ovflowd I didn't really get your idea, you want to have an
AvatarGrouponBlogHeaderwith list of contributor ? and set aside the modal?
No. I want AvatarGroup on Download page, that's what I wrote on my comment
(Also @richardlau I pinged you here because I recall you mentioning there was a way to get a list of all committers of a release, can you point me to the right direction?)
ð I don't recall that.
Oh, maybe I talked about that with someone else, but I recall our generated CHANGELOG (GitHub) contains the name of the commuters from their GitHub accounts, do you know where we do get that? Maybe cc @nodejs/releasers know?
(Also @richardlau I pinged you here because I recall you mentioning there was a way to get a list of all committers of a release, can you point me to the right direction?)
ð I don't recall that.
Oh, maybe I talked about that with someone else, but I recall our generated CHANGELOG (GitHub) contains the name of the commuters from their GitHub accounts, do you know where we do get that? Maybe cc @nodejs/releasers know?
changelog-maker pulls the information out of the git commits.
While examining the size of the page here, I noticed that all releases and changelogs were added to the page because we added the changelogs to the release in reducer. To reduce the size of the page, I have removed the changelog from the release and made it accessible via next-data ð
Source size:
89.19 KB
@canerakdas could you rebase? And @bmuenzenmeyer could you re-review?
@canerakdas I believe we need to reduce blur and add a close button. Also the changelog should only open after finishing fetching the data (IMO)
I think it looks better/accessible now, you can check it in the Vercel preview ð cc @ovflowd
Before:
After:
Before/Dark:
After/Dark:
Awesome! Would it also bee too much to ask you to check into what Richard mentioned (changelog-maker pulls the information out of the git commits.) for getting the full list of GitHub usernames for the "AvatarGroup" on the modal?
I believe we can merge this as it is; But we need reviews cc @bmuenzenmeyer @nodejs/nodejs-website
Ah, I completely missed this, I solved it by wrapping it with a button. If we think we may encounter such situations more often, it seems like we can use Radix primitive slot
Server `main`;
Due to the nature of our page structure, it is not clear exactly what these parts affect. There seems to be an increase only on `../app/[locale]/[[...path]]/page.js`.
Client `main`;
I'm not sure exactly how we can reduce the size here, but I'll check it out as well (if it's possible) ð