-
Notifications
You must be signed in to change notification settings - Fork 446
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’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: releases overview shows total docs in release #8232
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
2 Skipped Deployments
|
No changes to documentation |
Component Testing Report Updated Jan 16, 2025 2:10 PM (UTC) ❌ Failed Tests (1) -- expand for details
|
⚡️ Editor Performance ReportUpdated Thu, 16 Jan 2025 14:11:27 GMT
Detailed information🏠 Reference resultThe performance result of
🧪 Experiment resultThe performance result of this branch
📚 Glossary
|
aac22cd
to
b8f3e20
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test change isn't entirely relevant to the task at hand, but I came across it and thought it would be good to try and move as quickly as possible to using the release fixtures everywhere
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing existingDocumentsCount
from the store record
9045cb8
to
b6efb20
Compare
return ( | ||
<Translate | ||
t={t} | ||
i18nKey={documentCount > 1 ? `document-count_other` : `document-count_one`} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
our i18n tool actually handles these causes automatically, as far as I know you shouldn't need to do this :)
You need to send in the count like this (example on our code base, translation with count, use in component)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh well that's quite nice! There are a few other places I've done this (the wrong) thing. I'll remember to go back and update them.
Thanks for flagging. However clever of them!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great to me!
I guess the only thing would be about the translation key but otherwise everything looks good to go. My question is: are we going to add the separation of document changes (added, modified, deleted etc) at a later point or is that something that was dropped for now?
That's been dropped now. The document action will only be present on release dashboard lists, but since this requires some backend work in places to make performant, we aren't going to touch it until a later date. |
Description
ReleasesOverview
into separate components rather than all inside the column defsReleaseDocumentsCount
), to:documentsMetadata
count for active releasesfinalDocumentStates
count for published or archived releasesexistingDocumentCount
from the release metadata aggregator store as it's no longer neededWhat to review
Testing
Added tests to the cell components
Notes for release
N/A