feat(announcements): page timeline in announcements component#459
feat(announcements): page timeline in announcements component#459anabaarbosa wants to merge 5 commits into
Conversation
…ng and year-based timeline
… filtering and year-based timeline" This reverts commit f6ffa2a.
…in announcement components
anabaarbosa
added
release-no
✅ Deploy Preview for leafy-mooncake-7c2e5e ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
A few additions in this PR don't appear to be referenced anywhere — they'll become dead code on
The two unrelated fixes inside |
| /** Mesmas chaves que na main: strings literais dos tags no frontmatter (EN/ES/PT). */ | ||
| export const announcementTypeTagColorMap: Record<string, AnnouncementTagColor> = | ||
| { | ||
| 'New feature': 'Fixed', | ||
| Improvement: 'Closed', | ||
| 'Breaking change': 'Scheduled', | ||
| Deprecation: 'Gray', | ||
| Discontinuation: 'Gray', | ||
| 'Security update': 'Backlog', | ||
| 'Nueva funcionalidad': 'Fixed', | ||
| Mejora: 'Closed', | ||
| 'Cambio disruptivo': 'Scheduled', | ||
| Descontinuación: 'Gray', | ||
| 'Actualización de seguridad': 'Backlog', | ||
| 'Nova funcionalidade': 'Fixed', | ||
| Melhoria: 'Closed', | ||
| Descontinuação: 'Gray', | ||
| /** Grafia alternativa comum em conteúdo legado */ | ||
| Discontinuação: 'Gray', |
There was a problem hiding this comment.
Two things on this map:
-
Silent color change on existing content. On
main, the inline map insideAnnouncementCardhadDeprecation,Descontinuación, andDescontinuaçãoall mapping to the'Deprecation'pill color (pink). Here they're remapped to'Gray'. Is this intentional? It's the same color asSecurity update, I think it'd be best to have different colors. Maybe keep pink, unless it's already being used by some other badge. -
Verify the alt-spelling entries are needed. Standard Portuguese is
Descontinuação(withe);Discontinuação(withi) is a misspelling. Same idea on the EN side — the canonical EN type label intypeTagsByLocaleisDeprecation, notDiscontinuation. The comment says the alt spelling exists in legacy content, but I couldn't find any announcement using these strings as atagvalue — the only hits are inside announcement titles (e.g. "Discontinuation of …"), which aren't what this map keys off. Could you double-check against current frontmattertagsinhelp-center-content? If nothing uses them, dropping theDiscontinuationandDiscontinuaçãoentries keeps the map honest.
| } | ||
|
|
||
| const headerProps = hasSynopsis | ||
| ? { | ||
| role: 'button' as const, | ||
| tabIndex: 0, | ||
| 'aria-expanded': open, | ||
| onClick: () => setOpen((v) => !v), | ||
| onKeyDown: handleHeaderKeyDown, | ||
| } | ||
| : {} | ||
|
|
||
| return ( | ||
| <Flex sx={styles.row}> | ||
| <Box sx={styles.dateColumn}>{dateSideLabel}</Box> | ||
|
|
||
| <Flex sx={styles.trackColumn}> | ||
| <Box | ||
| sx={{ | ||
| ...styles.dot, | ||
| ...dotColors, | ||
| }} | ||
| /> |
There was a problem hiding this comment.
The whole header row is set up as an expand control, but the title inside it is also a link. That puts two clickable actions in one place — bad for screen readers and keyboard users (two tab stops per row, confusing labels).
The preview UX already feels right: caret expands, title opens the article. I think this is not a blocker, but I strongly recommend that you match the code to that behavior: make only the caret the expand control, and leave the title as a plain link.
|
|
||
| useEffect(() => { | ||
| setPage({ | ||
| curr: 1, | ||
| total: Math.ceil(filteredResult.length / itemsPerPage), | ||
| }) | ||
| }, [filteredResult]) | ||
| /** Lista após filtros — agrupamento só por ano (changelog). */ | ||
| const timelineAnnouncements = useMemo( | ||
| () => | ||
| filteredResult.map((announcement) => ({ | ||
| title: announcement.title, | ||
| publishedAt: new Date(announcement.createdAt), | ||
| articleLink: announcement.url, | ||
| synopsis: announcement.synopsis, | ||
| tags: announcement.tags, | ||
| })), | ||
| [filteredResult] | ||
| ) | ||
|
|
||
| const paginatedResult = useMemo(() => { | ||
| return filteredResult.slice( | ||
| (page.curr - 1) * itemsPerPage, | ||
| page.curr * itemsPerPage | ||
| ) | ||
| }, [page, filteredResult]) | ||
| const timelineByYear = useMemo(() => { | ||
| type TimelineItem = { | ||
| title: string | ||
| publishedAt: Date | ||
| articleLink: string | ||
| synopsis?: string | ||
| tags?: string[] | ||
| } | ||
|
|
||
| const bucket = new Map<string, TimelineItem[]>() |
There was a problem hiding this comment.
With pagination gone, the page now mounts every PUBLISHED/CHANGED announcement into the DOM on first paint (no virtualization). Announcements are append-only content, so this list only grows over time.
I am not sure this is the UX we want and maybe this will also impact performance over time. If you choose to merge this PR without addressing this, please create a github issue in this repo to 'restore announcements page pagination'.
|
|
||
| const trackColumn: SxStyleProp = { | ||
| width: '18px', | ||
| flexShrink: 0, |
There was a problem hiding this comment.
Minor/non-essential issue: A few raw hex values in this file (and in announcements-page.ts for the rail / year heading) look like they should be theme tokens: #9B9B9B (caret), #E0E0E0 (rail), #9AA3AE (year heading), #A1AAB7 (date column), #000711 (hover). You're already using muted.0 for releaseTitle and body, so the mix is inconsistent. Future theme adjustments will have to chase the literals. Where a brand-ui token exists, please use it. Where it doesn't yet, it's worth adding to the theme rather than baking the literal here.
| @@ -0,0 +1,160 @@ | |||
| import { SxStyleProp } from '@vtex/brand-ui' | |||
|
|
|||
| /** Linha inteira: data | marcador | conteúdo (tronco único via rail absoluto na página). */ | |||
There was a problem hiding this comment.
Minor/non-essential issue: The new styles files (announcement-expandable-row/styles.ts, announcement-timeline-card/styles.ts, announcements-page.ts) carry PT-only comments — e.g. /** Linha inteira: data | marcador | conteúdo (tronco único via rail absoluto na página). */, /** Trilho do ano só entre as linhas (eixo alinhado à coluna de pontos). */. The rest of the codebase uses EN for code comments, so it's worth translating these for consistency.
There was a problem hiding this comment.
Thanks for the timeline UX work — the new layout is solid and centralizing the tag→color mapping in announcementTypeTags.ts is a nice cleanup.
Requesting changes for a few things before merge — left inline comments with details:
- Blocking: a meaningful amount of dead code (unused new util files, an unused i18n key in each locale, and 6 new optional props on
AnnouncementTimelineCardwhose new render branch is unreachable). See the general comment. - Suggestions: silent color regression on the
Deprecationtag (pink → gray on the existing landing-page card too); accessibility issue with the title<a>nested inside arole="button"row; and a heads-up that removing pagination renders the full announcements list unbounded. - Nits: hardcoded hex colors instead of theme tokens; PT-only code comments in the new styles files.
2 participants
What is the purpose of this pull request?
Update to the Announcements homepage: now displays a single list with filters and search, grouped by year, with a changelog-style layout, month and day on the left, vertical line only in each year's block, expandable lines for synopsis. New
AnnouncementExpandableRowandannouncementTypeTagsutility (EN/PT/ES) centering marker colors and type on the timeline.Screenshots or example usage
Types of changes