feat(org): add Org Lens Meetings page list only UI#1043
Merged
Conversation
Implement Meetings page for Org Lens with demo data seam. Includes KPI strip, combined tab/filter bar (Upcoming/Past/Pending RSVP + Search + All Projects + All Types), and per-tab card lists. Upcoming card matches app.lfx.dev/meetings: privacy badge, recurrence, 3 action icons, date chip, artifact text-links, agenda show-more, Resources/People-Invited two-column layout with progress bar and company invitees (Org Lens addition from prototype), Guests/Details footer buttons. Past card shows attendance bar, artifact pills, and org attendance. Pending RSVP shows compact rows with RSVP actions. Signed-off-by: daniel qualls <dqualls@linuxfoundation.org>
Guard window/navigator calls in copyLink and openJoin with isPlatformBrowser to prevent SSR crashes. Replace plain Set with signal<ReadonlySet<string>> for expandedIds so show-more/less toggles trigger change detection in zoneless mode. Signed-off-by: daniel qualls <dqualls@linuxfoundation.org>
The button label says See Meeting Details but was wired to openJoin which opens a join URL — a label/action mismatch. Remove the click handler and the unused openJoin method; the button is a no-op placeholder until meeting detail routing is wired. Signed-off-by: daniel qualls <dqualls@linuxfoundation.org>
Replace OrgPendingRsvpMeeting duplicate field declarations with Pick<OrgMeeting, ...> to avoid interface drift. Replace stale Date.now() captured at construction time with a private hoursUntil() helper that reads Date.now() at call time, fixing stale deadline chips in SSR and long-lived sessions. Also guard negative hours with 'Starting soon' label. Signed-off-by: daniel qualls <dqualls@linuxfoundation.org>
Top-level constant expressions using Date.now() execute once at SSR module load time and produce stale timestamps for all subsequent users. Replace with a static ISO string so the demo date is deterministic. Signed-off-by: daniel qualls <dqualls@linuxfoundation.org>
…base Conflict resolution during rebase accidentally dropped the project-staff.constants barrel export, breaking PROJECT_STAFF_ROWS consumers. Restore it alongside the org-meetings.constants export. Signed-off-by: daniel qualls <dqualls@linuxfoundation.org>
Signed-off-by: daniel qualls <dqualls@linuxfoundation.org>
Move hoursUntil private helper after all protected methods to satisfy the typescript-eslint/member-ordering rule. Signed-off-by: daniel qualls <dqualls@linuxfoundation.org>
…meetings - Filter bar is now a standalone lfx-card (was nested inside shared content card) - Upcoming and past meeting cards render directly in page flex column with gap-6 - Remove outer lfx-card wrapper that previously wrapped filter bar + tab content - Remove Pending RSVP tab and All Projects filter (not on live site) - Past meeting cards converted from divide-y list to individual bordered cards - OrgMeetingsTabId union type narrowed from 'upcoming'|'past'|'pending' to 'upcoming'|'past' - Drop filteredPending, pendingMeetings, filterProject, onRsvp, and OrgPendingRsvpMeetingsComponent LFXV2-1901 LFXV2-1902 LFXV2-1903 LFXV2-1904 Signed-off-by: daniel qualls <dqualls@linuxfoundation.org>
- Filter bar tabs show plain labels (no counts) matching app.lfx.dev/meetings - Search bar fills center with flex-1; All Types pushed to right - Meeting cards use white bg + shadow-sm matching live site card style - Upcoming cards show type badge (Board=violet, Working Group=blue) - Upcoming cards show only copy icon (edit/delete removed) LFXV2-1901 Signed-off-by: daniel qualls <dqualls@linuxfoundation.org>
- KPI strip switches per tab: past shows Past Meetings + Recordings Available - Inactive tab pill uses bg-gray-100 (no border) to match live site - Upcoming cards: Private badge uses shield-heart icon + rounded-full; no badge for public meetings; Board/WG = blue border pill; Other = gray border pill; recurrence shown as border pill; status artifacts inline on date row; right column = RSVP (Yes/No/Maybe) for private, Register for public + See Meeting Details; removed Guests/Details bottom row - Past cards simplified to match live site: privacy/type badges + title + date + inline artifacts + Resources | See Meeting Details only; removed attendance bar, artifact section, org invitees LFXV2-1901 Signed-off-by: daniel qualls <dqualls@linuxfoundation.org>
…ator - Use fa-circle-dot for Recording (not fa-circle-video) - Use en-dash (–) in date time range to match live site - Align AI Summary to emerald-500 matching live site color LFXV2-1901 Signed-off-by: daniel qualls <dqualls@linuxfoundation.org>
- Artifact chips (Recording/Transcripts/AI Summary/Minutes) now use colored background pills (bg-red-50, bg-blue-50, bg-emerald-50) with matching light borders to match live site - Other type badge uses bg-gray-100 with no border (not gray border) - Recurrence pill uses bg-gray-100 with no border LFXV2-1901 Signed-off-by: daniel qualls <dqualls@linuxfoundation.org>
…ressed to tab buttons - Remove unused tabPillOptions computed signal and initTabPillOptions() method; template uses the tabs constant directly - Remove now-unused FilterPillOption import - Add [attr.aria-pressed] to tab toggle buttons for screen reader support LFXV2-1901 Signed-off-by: daniel qualls <dqualls@linuxfoundation.org>
…utton aria-label Delete org-pending-rsvp-meetings component files left on disk after the Pending RSVP tab was removed in a12399b. Replace title= with aria-label= on icon-only copy buttons in upcoming and past meeting cards. LFXV2-1901 Signed-off-by: daniel qualls <dqualls@linuxfoundation.org>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds the Org Meetings dashboard to Org Lens with shared meeting data types, a routed meetings page with tab and filter state, upcoming and past meeting views, sidebar navigation, and Playwright coverage. ChangesOrg Meetings Dashboard
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
danoqualls
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
Implements a new Org Lens “Meetings” page under /org/meetings, including demo-data-driven upcoming/past tabs, KPI strip, and a shared filter bar, and re-enables the org sidebar navigation entry for Meetings.
Changes:
- Added shared Org Meetings interfaces + demo constants for tabs, KPIs, meeting records, and filter options.
- Introduced new Org Meetings dashboard component and tab subcomponents (Upcoming/Past) with filtering, skeleton/empty states, and card layouts.
- Wired the
/org/meetingsroute and restored the Meetings nav entry in the org sidebar.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/shared/src/interfaces/org-meetings.interface.ts | Adds shared types for Org Meetings page data modeling (tabs, meetings, artifacts, RSVP/attendance). |
| packages/shared/src/interfaces/index.ts | Re-exports Org Meetings interfaces from the shared interfaces barrel. |
| packages/shared/src/constants/org-meetings.constants.ts | Introduces Org Meetings demo data and constants (tabs, KPIs, filter options, demo meeting lists). |
| packages/shared/src/constants/index.ts | Re-exports Org Meetings constants from the shared constants barrel. |
| apps/lfx-one/src/app/modules/dashboards/org/org-meetings/org-meetings.component.ts | Adds the Org Meetings page container with query-param tab state + search/type filtering. |
| apps/lfx-one/src/app/modules/dashboards/org/org-meetings/org-meetings.component.html | Implements the Org Meetings page layout (header, KPI strip, tab/filter bar, tab content). |
| apps/lfx-one/src/app/modules/dashboards/org/org-meetings/components/org-upcoming-meetings/org-upcoming-meetings.component.ts | Adds Upcoming tab logic (expand/collapse + copy link handler). |
| apps/lfx-one/src/app/modules/dashboards/org/org-meetings/components/org-upcoming-meetings/org-upcoming-meetings.component.html | Renders Upcoming meeting cards including badges, agenda expand, resources, and RSVP/register layout. |
| apps/lfx-one/src/app/modules/dashboards/org/org-meetings/components/org-past-meetings/org-past-meetings.component.ts | Adds Past tab logic (copy link handler). |
| apps/lfx-one/src/app/modules/dashboards/org/org-meetings/components/org-past-meetings/org-past-meetings.component.html | Renders Past meeting cards including artifacts, resources, and “load older meetings” CTA. |
| apps/lfx-one/src/app/layouts/main-layout/main-layout.component.ts | Re-enables Meetings in the org sidebar navigation. |
| apps/lfx-one/src/app/app.routes.ts | Routes /org/meetings to the new OrgMeetingsComponent (removes prior redirect-to-overview). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
🚀 Deployment StatusYour branch has been deployed to: https://ui-pr-1043.dev.v2.cluster.linuxfound.info Deployment Details:
The deployment will be automatically removed when this PR is closed. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
packages/shared/src/constants/org-meetings.constants.ts (1)
56-57: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
OrgMeetingTypefor the valid-type set.
ReadonlySet<string>weakens the shared contract here. IfOrgMeetingTypechanges later, this validator can drift without a compile-time failure. Type the set asReadonlySet<OrgMeetingType>(or derive it from a typed tuple) so the constants stay locked to the interface.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/shared/src/constants/org-meetings.constants.ts` around lines 56 - 57, The VALID_ORG_MEETING_TYPE_VALUES constant is typed too loosely and can drift from the OrgMeetingType contract. Update the constant in org-meetings.constants so the set is typed as ReadonlySet<OrgMeetingType> (or build it from a typed tuple) and keep the values aligned with OrgMeetingType, ensuring compile-time failures if the shared type changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/lfx-one/src/app/modules/dashboards/org/org-meetings/components/org-past-meetings/org-past-meetings.component.html`:
- Around line 41-47: The action buttons in org-past-meetings.component.html
remove the default focus outline via focus-visible:outline-none without
providing a visible replacement. Update the copy, resource, and details buttons
in this template to include a clear keyboard focus style (for example a ring or
outline state) while preserving the existing hover behavior, and make sure the
same fix is applied to the other affected buttons referenced in the component
markup.
- Around line 91-96: The resource button, the “See Meeting Details” button, and
the “Load older meetings” CTA in org-past-meetings.component.html are currently
dead controls with no bound behavior. Wire each control to a real action in the
corresponding org-past-meetings component by adding the appropriate click
handlers or navigation bindings, and connect them to existing methods or add new
ones in org-past-meetings.component.ts so the UI elements actually open the
resource, show meeting details, and load older meetings before exposing them.
In
`@apps/lfx-one/src/app/modules/dashboards/org/org-meetings/components/org-upcoming-meetings/org-upcoming-meetings.component.html`:
- Around line 46-53: Restore a visible keyboard focus indicator for the action
buttons in org-upcoming-meetings.component.html. The current button controls
(copy, expand, resource, RSVP/Register, and details) use
focus-visible:outline-none without a replacement style, so add a clear
focus-visible state (for example via an existing Tailwind focus class pattern)
on the relevant button elements. Update the interactive elements in the
org-upcoming-meetings template so their focus state is visible and consistent
while keeping the existing click handlers like copyLink and the other action
bindings intact.
- Around line 106-111: The CTA buttons in org-upcoming-meetings.component.html
are rendered as interactive controls without any action, so update the resource,
RSVP/Register, and meeting-details elements to either wire them to a real
click/routerLink/href behavior or mark them disabled/non-interactive. Use the
existing template blocks around the resource button and the other CTA sections
in the same component to either add the appropriate handlers/links or change
them to plain text so users are not misled by inert buttons.
In
`@apps/lfx-one/src/app/modules/dashboards/org/org-meetings/org-meetings.component.html`:
- Around line 12-29: The KPI strip and the filter bar are currently fixed to
desktop-only layouts, which can squeeze or overflow on smaller screens. Update
the org meetings template to add responsive breakpoints to the KPI grid and the
tabs/search/filter row so they stack or wrap appropriately on mobile and tablet
sizes. Use the existing template sections in org-meetings.component.html
(including the KPI card loop and the filter bar markup) as the place to apply
responsive Tailwind classes and keep the desktop layout unchanged.
---
Nitpick comments:
In `@packages/shared/src/constants/org-meetings.constants.ts`:
- Around line 56-57: The VALID_ORG_MEETING_TYPE_VALUES constant is typed too
loosely and can drift from the OrgMeetingType contract. Update the constant in
org-meetings.constants so the set is typed as ReadonlySet<OrgMeetingType> (or
build it from a typed tuple) and keep the values aligned with OrgMeetingType,
ensuring compile-time failures if the shared type changes.
🪄 Autofix (Beta)
❌ Autofix failed (check again to retry)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 11633b49-8c75-4663-9dfe-0004d4dc3d2e
📒 Files selected for processing (12)
apps/lfx-one/src/app/app.routes.tsapps/lfx-one/src/app/layouts/main-layout/main-layout.component.tsapps/lfx-one/src/app/modules/dashboards/org/org-meetings/components/org-past-meetings/org-past-meetings.component.htmlapps/lfx-one/src/app/modules/dashboards/org/org-meetings/components/org-past-meetings/org-past-meetings.component.tsapps/lfx-one/src/app/modules/dashboards/org/org-meetings/components/org-upcoming-meetings/org-upcoming-meetings.component.htmlapps/lfx-one/src/app/modules/dashboards/org/org-meetings/components/org-upcoming-meetings/org-upcoming-meetings.component.tsapps/lfx-one/src/app/modules/dashboards/org/org-meetings/org-meetings.component.htmlapps/lfx-one/src/app/modules/dashboards/org/org-meetings/org-meetings.component.tspackages/shared/src/constants/index.tspackages/shared/src/constants/org-meetings.constants.tspackages/shared/src/interfaces/index.tspackages/shared/src/interfaces/org-meetings.interface.ts
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. The branch was updated while autofix was in progress. Please try again. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
apps/lfx-one/src/app/modules/dashboards/org/org-meetings/org-meetings.component.html (2)
40-45: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winMissing
focus-visiblering styles on tab/toggle pill buttons.Per the PR's stated fix of adding
focus-visiblering styles to meeting card buttons for accessibility, these tab and pending-RSVP toggle buttons appear to lack the same treatment, leaving no visible keyboard-focus indicator.Also applies to: 58-69
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/lfx-one/src/app/modules/dashboards/org/org-meetings/org-meetings.component.html` around lines 40 - 45, The tab and pending-RSVP pill buttons in org-meetings.component.html are missing the same focus-visible accessibility treatment added elsewhere. Update the button class bindings for the tab selector and the pending-RSVP toggle to include a visible focus-visible ring/outline state, using the existing active/inactive styling in the button templates so keyboard focus is clearly indicated.
38-45: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract duplicated pill-class ternary logic.
The active/inactive pill class strings are duplicated near-verbatim between the tab buttons and the pending-RSVP toggle button. Extracting a small helper (e.g., a
pillClass(active: boolean): stringmethod or template function) would avoid divergence if the styling changes later.♻️ Suggested direction
- [class]=" - activeTab() === tab.id - ? 'px-3 py-1 rounded-full text-sm transition-colors bg-blue-500 text-white' - : 'px-3 py-1 rounded-full text-sm transition-colors bg-gray-100 text-gray-700 hover:bg-gray-200' - " + [class]="pillClass(activeTab() === tab.id)" ... - [class]=" - pendingRsvpOnly() - ? 'px-3 py-1 rounded-full text-sm transition-colors bg-blue-500 text-white' - : 'px-3 py-1 rounded-full text-sm transition-colors bg-gray-100 text-gray-700 hover:bg-gray-200' - " + [class]="pillClass(pendingRsvpOnly())"protected pillClass(active: boolean): string { return active ? 'px-3 py-1 rounded-full text-sm transition-colors bg-blue-500 text-white' : 'px-3 py-1 rounded-full text-sm transition-colors bg-gray-100 text-gray-700 hover:bg-gray-200'; }Also applies to: 55-69
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/lfx-one/src/app/modules/dashboards/org/org-meetings/org-meetings.component.html` around lines 38 - 45, The pill-style active/inactive class ternary is duplicated in the org meetings template, so extract it into a shared helper on org-meetings.component.ts (for example, a pillClass(active: boolean) method) and use that from both the tab button and the pending-RSVP toggle button. Keep the existing class strings centralized in the helper so any future styling change only needs to be made once.apps/lfx-one/src/app/modules/dashboards/org/org-meetings/components/org-past-meetings/org-past-meetings.component.html (2)
41-47: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winNo success/failure feedback on copy action.
Same gap as the upcoming-meetings card:
cdkCopyToClipboardis bound without(cdkCopyToClipboardCopied), so users get no confirmation the link was actually copied.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/lfx-one/src/app/modules/dashboards/org/org-meetings/components/org-past-meetings/org-past-meetings.component.html` around lines 41 - 47, The copy button in org-past-meetings.component.html uses cdkCopyToClipboard without any copied-state handling, so users get no confirmation. Add a cdkCopyToClipboardCopied handler on the existing button bound to meetingLinkUrl(meeting.id), and wire it to the same success feedback pattern used in the upcoming-meetings card so the copy action visibly confirms success.
89-96: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
track resourcerisks duplicate-key collisions.Same concern as in the upcoming meetings template — tracking by the raw resource string can collide if two resources share a name. Prefer index or a stable id.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/lfx-one/src/app/modules/dashboards/org/org-meetings/components/org-past-meetings/org-past-meetings.component.html` around lines 89 - 96, The `@for` loop in `org-past-meetings.component.html` is tracking resources by the raw `resource` string, which can cause duplicate-key collisions if names repeat. Update the tracking expression in the `meeting.resources` loop to use a safer unique key, such as the item index or a stable resource identifier if one exists, matching the approach used in the other meetings template.apps/lfx-one/src/app/modules/dashboards/org/org-meetings/components/org-past-meetings/org-past-meetings.component.ts (1)
20-26: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate
meetingLinkUrlimplementation.This method is byte-for-byte identical to
OrgUpcomingMeetingsComponent.meetingLinkUrl(same SSR guard, same comment, same URL construction). Consider extracting a shared helper (e.g. a small pure function or an injectable service) so the SSR-link-building logic has a single source of truth instead of being maintained in two places.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/lfx-one/src/app/modules/dashboards/org/org-meetings/components/org-past-meetings/org-past-meetings.component.ts` around lines 20 - 26, The meetingLinkUrl logic in OrgPastMeetingsComponent is duplicated from OrgUpcomingMeetingsComponent, so move the SSR-safe URL building into a shared helper or small service and have both components call that single implementation. Keep the same isPlatformBrowser guard and relative-path fallback, but centralize the path/origin construction so the copy button link generation has one source of truth.apps/lfx-one/src/app/modules/dashboards/org/org-meetings/components/org-upcoming-meetings/org-upcoming-meetings.component.ts (1)
43-68: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winTemplate-bound methods recompute on every CD cycle for every row.
meetingLinkUrl,totalInvited,attendingPercent,rsvpPercent, andrsvpBadgeare all plain methods invoked directly from template bindings inside the@forloop (several multiple times per row, e.g.totalInvitedis called independently in three different places for the same meeting). In zoneless Angular this re-executes for every meeting card whenever any signal in the tree changes (e.g. toggling one card'sexpandedIds), not just the affected row. This mirrors the exact anti-pattern already flagged and fixed forisExpanded()in this same component (calls should be signal-driven rather than method-driven inside@for).Consider deriving these once per meeting (e.g. a
computed()that mapsmeetings()into a view-model array with pre-computedlinkUrl,totalInvited,attendingPercent, and badge info) instead of recomputing them from the template.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/lfx-one/src/app/modules/dashboards/org/org-meetings/components/org-upcoming-meetings/org-upcoming-meetings.component.ts` around lines 43 - 68, The template is repeatedly calling plain methods from inside the `@for` loop, causing all meeting rows to recompute on every change detection pass. Move the logic in meetingLinkUrl, totalInvited, attendingPercent, rsvpPercent, and rsvpBadge out of direct template bindings by deriving a per-meeting view model with computed() so each meeting’s link, totals, percentages, and badge are calculated once and reused. Use the existing org-upcoming-meetings.component.ts class and its meetings()/expandedIds pattern as the place to precompute these values instead of invoking methods from the template.apps/lfx-one/src/app/modules/dashboards/org/org-meetings/components/org-upcoming-meetings/org-upcoming-meetings.component.html (2)
106-117: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
track resourcerisks duplicate-key collisions.Resources are tracked by their raw string value. If two resources in the same meeting share an identical name/label, Angular's
@forwill treat them as the same tracked item, causing incorrect DOM reuse. Prefer tracking by index or a stable resource id if one exists on the model.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/lfx-one/src/app/modules/dashboards/org/org-meetings/components/org-upcoming-meetings/org-upcoming-meetings.component.html` around lines 106 - 117, The `@for` in org-upcoming-meetings.component.html is tracking resources by the raw string value, which can collide when duplicate labels exist in meeting.resources. Update the tracking expression in the resources loop to use a unique stable identifier if the model provides one, or fall back to the item index so each resource is rendered distinctly.
46-51: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winHandle the clipboard result
cdkCopyToClipboardemits the copy outcome, but nothing listens to it here. Bind(cdkCopyToClipboardCopied)and surface success/failure so permission-denied failures aren’t silent.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/lfx-one/src/app/modules/dashboards/org/org-meetings/components/org-upcoming-meetings/org-upcoming-meetings.component.html` around lines 46 - 51, The copy action in orgUpcomingMeetingsComponent is missing handling for the clipboard result, so failures can be silent. Update the button that uses cdkCopyToClipboard in org-upcoming-meetings.component.html to bind (cdkCopyToClipboardCopied) and route the outcome into orgUpcomingMeetingsComponent so you can surface a success or failure state (for example via an existing notification or handler method). Use the meetingLinkUrl(meeting.id) binding and the copy button’s existing data-testid/aria-label to locate the control.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/lfx-one/src/app/modules/dashboards/org/org-meetings/components/org-upcoming-meetings/org-upcoming-meetings.component.html`:
- Around line 154-174: The invitee list in org-upcoming-meetings.component.html
is tracking rows by invitee.name, which can collide when names repeat. Update
the `@for` in the orgInvitees block to use a stable unique identifier from
OrgMeetingInvitee instead, and if one does not exist, add an id/email field to
the model and bind the track expression to that unique value.
---
Nitpick comments:
In
`@apps/lfx-one/src/app/modules/dashboards/org/org-meetings/components/org-past-meetings/org-past-meetings.component.html`:
- Around line 41-47: The copy button in org-past-meetings.component.html uses
cdkCopyToClipboard without any copied-state handling, so users get no
confirmation. Add a cdkCopyToClipboardCopied handler on the existing button
bound to meetingLinkUrl(meeting.id), and wire it to the same success feedback
pattern used in the upcoming-meetings card so the copy action visibly confirms
success.
- Around line 89-96: The `@for` loop in `org-past-meetings.component.html` is
tracking resources by the raw `resource` string, which can cause duplicate-key
collisions if names repeat. Update the tracking expression in the
`meeting.resources` loop to use a safer unique key, such as the item index or a
stable resource identifier if one exists, matching the approach used in the
other meetings template.
In
`@apps/lfx-one/src/app/modules/dashboards/org/org-meetings/components/org-past-meetings/org-past-meetings.component.ts`:
- Around line 20-26: The meetingLinkUrl logic in OrgPastMeetingsComponent is
duplicated from OrgUpcomingMeetingsComponent, so move the SSR-safe URL building
into a shared helper or small service and have both components call that single
implementation. Keep the same isPlatformBrowser guard and relative-path
fallback, but centralize the path/origin construction so the copy button link
generation has one source of truth.
In
`@apps/lfx-one/src/app/modules/dashboards/org/org-meetings/components/org-upcoming-meetings/org-upcoming-meetings.component.html`:
- Around line 106-117: The `@for` in org-upcoming-meetings.component.html is
tracking resources by the raw string value, which can collide when duplicate
labels exist in meeting.resources. Update the tracking expression in the
resources loop to use a unique stable identifier if the model provides one, or
fall back to the item index so each resource is rendered distinctly.
- Around line 46-51: The copy action in orgUpcomingMeetingsComponent is missing
handling for the clipboard result, so failures can be silent. Update the button
that uses cdkCopyToClipboard in org-upcoming-meetings.component.html to bind
(cdkCopyToClipboardCopied) and route the outcome into
orgUpcomingMeetingsComponent so you can surface a success or failure state (for
example via an existing notification or handler method). Use the
meetingLinkUrl(meeting.id) binding and the copy button’s existing
data-testid/aria-label to locate the control.
In
`@apps/lfx-one/src/app/modules/dashboards/org/org-meetings/components/org-upcoming-meetings/org-upcoming-meetings.component.ts`:
- Around line 43-68: The template is repeatedly calling plain methods from
inside the `@for` loop, causing all meeting rows to recompute on every change
detection pass. Move the logic in meetingLinkUrl, totalInvited,
attendingPercent, rsvpPercent, and rsvpBadge out of direct template bindings by
deriving a per-meeting view model with computed() so each meeting’s link,
totals, percentages, and badge are calculated once and reused. Use the existing
org-upcoming-meetings.component.ts class and its meetings()/expandedIds pattern
as the place to precompute these values instead of invoking methods from the
template.
In
`@apps/lfx-one/src/app/modules/dashboards/org/org-meetings/org-meetings.component.html`:
- Around line 40-45: The tab and pending-RSVP pill buttons in
org-meetings.component.html are missing the same focus-visible accessibility
treatment added elsewhere. Update the button class bindings for the tab selector
and the pending-RSVP toggle to include a visible focus-visible ring/outline
state, using the existing active/inactive styling in the button templates so
keyboard focus is clearly indicated.
- Around line 38-45: The pill-style active/inactive class ternary is duplicated
in the org meetings template, so extract it into a shared helper on
org-meetings.component.ts (for example, a pillClass(active: boolean) method) and
use that from both the tab button and the pending-RSVP toggle button. Keep the
existing class strings centralized in the helper so any future styling change
only needs to be made once.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fed33e03-2aba-4818-a900-b0011a047989
📒 Files selected for processing (11)
apps/lfx-one/src/app/modules/dashboards/org/org-meetings/components/org-past-meetings/org-past-meetings.component.htmlapps/lfx-one/src/app/modules/dashboards/org/org-meetings/components/org-past-meetings/org-past-meetings.component.tsapps/lfx-one/src/app/modules/dashboards/org/org-meetings/components/org-upcoming-meetings/org-upcoming-meetings.component.htmlapps/lfx-one/src/app/modules/dashboards/org/org-meetings/components/org-upcoming-meetings/org-upcoming-meetings.component.tsapps/lfx-one/src/app/modules/dashboards/org/org-meetings/org-meetings.component.htmlapps/lfx-one/src/app/modules/dashboards/org/org-meetings/org-meetings.component.tsapps/lfx-one/src/app/shared/components/stat-card-grid/stat-card-grid.component.htmlpackages/shared/src/constants/org-meetings.constants.tspackages/shared/src/interfaces/index.tspackages/shared/src/interfaces/org-meetings.interface.tspackages/shared/src/interfaces/stat-card.interface.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/shared/src/interfaces/index.ts
- packages/shared/src/interfaces/org-meetings.interface.ts
Address review comments from copilot-pull-request-reviewer[bot]: - org-upcoming-meetings.component.html/.ts, org-past-meetings.component.html/.ts: wrapped the copy-link button in @defer (on idle), matching the dashboard-meeting-card pattern, and reworded the SSR-guard comment so it stays accurate whether the button renders eagerly or deferred - org-meetings.component.ts: filtered the Upcoming list to startTime >= now and the Past list to startTime < now, so tabs can't show meetings that have already started/aren't actually in the past - org-upcoming-meetings.component.html: track invitees by $index instead of invitee.name, since name isn't guaranteed unique or stable - org-meetings-dashboard.spec.ts: added e2e coverage for the search filter and the Pending RSVP toggle narrowing the upcoming list No code change (explained in PR description instead): - resource links stay plain text pending a real attachment/document model - PR description reworded to describe the actual Guests/See Details CTAs instead of RSVP/Register, which aren't part of this list-only pass Resolves 10 review threads. Signed-off-by: daniel qualls <dqualls@linuxfoundation.org>
Address review comments from Copilot: - org-meetings.component.html: replace hand-rolled KPI markup with the shared lfx-stat-card-grid component (now supports a configurable 2-column layout) so subLine rendering isn't maintained in two places - org-upcoming-meetings.component.ts, org-past-meetings.component.ts: extract the duplicated SSR-safe meetingLinkUrl guard into a shared toAbsoluteUrl() util in @lfx-one/shared/utils - org-meetings.constants.ts: derive demo meeting startTime/endTime relative to the current date instead of hardcoded absolute dates, so the Upcoming/Past demo data and dependent e2e assertions stay valid over time Resolves 4 review threads. Signed-off-by: daniel qualls <dqualls@linuxfoundation.org>
The multi-line div wrap in stat-card-grid.component.html failed yarn format:check on PR #1043's CI. Signed-off-by: daniel qualls <dqualls@linuxfoundation.org>
audigregorie
reviewed
Jul 2, 2026
audigregorie
left a comment
audigregorie
left a comment
Contributor
There was a problem hiding this comment.
Code Review Summary
Solid Org Lens Meetings list-only UI with thoughtful demo-data seam (demoMeetingTimes), good a11y on tab pills and copy buttons, and Playwright coverage for core tab/search/RSVP flows. Remaining gaps are mostly placeholder CTAs and KPI values that do not match the visible demo list.
Major — outside the diff
- Branch hygiene:
main..HEADspans 73 files and 40+ commits (many merge commits and auto-generated "Potential fix for pull request finding" messages). Consider squashing or rebasing to the org-meetings feature commits before merge so reviewers can focus on LFXV2-1901 scope.
Minor — outside the diff
- E2E gaps: New spec covers tab switching, search, and Pending RSVP toggle but not project/type filters or Past-tab KPI assertions listed in the PR test plan.
What's done well
- Shared types/constants split (
OrgMeetingBase/OrgPastMeeting) and relative demo timestamps keep e2e stable. - Clipboard copy is SSR-safe via
@defer (on idle)+toAbsoluteUrlwithisPlatformBrowser. - Filter bar uses
aria-pressed, sr-only labels, anddata-testidhooks consistently.
- org-meetings.component.ts: derive KPI upcoming/past counts from filteredUpcoming()/filteredPast().length instead of static demo constants, so the strip matches the rendered list - org-meetings.component.ts: reset pendingRsvpOnly when switching away from the upcoming tab so the filter doesn't leak across tabs - org-upcoming-meetings.component.html: disable non-functional Guests/ See Details/See Meeting Details buttons with aria-disabled + muted styling instead of leaving them clickable with no handler - org-past-meetings.component.html: disable non-functional See Meeting Details and Load older meetings buttons the same way - org-upcoming-meetings.component.html: use a composite name+title track key for orgInvitees instead of $index, since invitees don't reorder and a stable key avoids unnecessary re-renders - org-upcoming-meetings.component.ts: move RSVP_BADGES/NO_RESPONSE_BADGE module-level consts into packages/shared/src/constants per the no-local-consts source hygiene rule Resolves 8 review threads. Signed-off-by: daniel qualls <dqualls@linuxfoundation.org>
audigregorie
approved these changes
Jul 2, 2026
🧹 Deployment RemovedThe deployment for PR #1043 has been removed. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Business case: UI only and list view only of Meetings that an organization's employees are involved in.
This pull request introduces the new Organization Meetings feature to the application, making the Meetings page and navigation visible and functional. It adds UI components for viewing upcoming and past meetings, complete with meeting details, filters, and resource links. The changes also update the organization dashboard navigation and routes to enable direct access to the Meetings page.
Feature: Organization Meetings Page
app.routes.ts,main-layout.component.ts) [1] [2]org-meetingscomponent, which includes a page header, KPI summary, filter bar (tabs, search, type filter), and tabbed views for upcoming and past meetings. (org-meetings.component.html)UI Components: Upcoming and Past Meetings
OrgUpcomingMeetingsComponentandOrgPastMeetingsComponentto display lists of upcoming and past meetings, respectively, each with skeleton loaders, empty states, and detailed meeting cards. (org-upcoming-meetings.component.ts/.html,org-past-meetings.component.ts/.html) [1] [2] [3] [4]Navigation and Routing Updates
app.routes.ts,main-layout.component.ts) [1] [2]Summary
/org/meetingsroute and re-enables the Meetings nav entry in the org sidebarRemoved
openJoinbutton (label/action mismatch — placeholder until meeting detail routing is wired)string[], no URLs) — clickable resource links are follow-up work pending a real attachment/document modelTest plan
/org/meetings— page loads with KPI strip and Upcoming tab activearia-pressedstate for screen readersloading=true🤖 Generated with Claude Code