feat(environments): make environment list columns sortable - #3020
Open
RemiBonnet wants to merge 1 commit into
Open
RemiBonnet wants to merge 1 commit into
RemiBonnet wants to merge 1 commit into
Conversation
|
View your CI Pipeline Execution ↗ for commit 1852c31
💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗ ☁️ Nx Cloud last updated this comment at |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
2 issues found across 2 files
Confidence score: 3/5
- In
libs/domains/environments/feature/src/lib/environments-table/environment-section/environment-section.spec.tsx, the unanchored role query also matches row links, so the assertion receives four links and the test fails — scope the query to the individual environment link. - In
libs/domains/environments/feature/src/lib/environments-table/environment-section/environment-section.tsx, Last update sorting treats a missingupdated_atas 1970 even though the row displays “just now,” so the sort order can disagree with what users see — use the display’sDate.now()fallback.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="libs/domains/environments/feature/src/lib/environments-table/environment-section/environment-section.spec.tsx">
<violation number="1" location="libs/domains/environments/feature/src/lib/environments-table/environment-section/environment-section.spec.tsx:171">
P2: This unanchored role query also matches each row’s `role="link"` in addition to its nested environment anchor, so the assertion receives four links and the new test fails. Restrict the accessible name to the individual environment anchors.</violation>
</file>
<file name="libs/domains/environments/feature/src/lib/environments-table/environment-section/environment-section.tsx">
<violation number="1" location="libs/domains/environments/feature/src/lib/environments-table/environment-section/environment-section.tsx:196">
P3: The Last update sort disagrees with the row display when `updated_at` is missing: the row shows “just now,” but this comparator sorts it as 1970. Use the same `Date.now()` fallback as the display.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| ) | ||
|
|
||
| const getEnvironmentNames = () => | ||
| screen.getAllByRole('link', { name: /alpha|beta/i }).map((link) => link.textContent) |
Contributor
There was a problem hiding this comment.
P2: This unanchored role query also matches each row’s role="link" in addition to its nested environment anchor, so the assertion receives four links and the new test fails. Restrict the accessible name to the individual environment anchors.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At libs/domains/environments/feature/src/lib/environments-table/environment-section/environment-section.spec.tsx, line 171:
<comment>This unanchored role query also matches each row’s `role="link"` in addition to its nested environment anchor, so the assertion receives four links and the new test fails. Restrict the accessible name to the individual environment anchors.</comment>
<file context>
@@ -157,6 +157,45 @@ describe('EnvironmentSection', () => {
+ )
+
+ const getEnvironmentNames = () =>
+ screen.getAllByRole('link', { name: /alpha|beta/i }).map((link) => link.textContent)
+
+ expect(getEnvironmentNames()).toEqual(['Alpha', 'Beta'])
</file context>
Suggested change
| screen.getAllByRole('link', { name: /alpha|beta/i }).map((link) => link.textContent) | |
| screen.getAllByRole('link', { name: /^(Alpha|Beta)$/i }).map((link) => link.textContent) |
| } | ||
|
|
||
| function lastUpdateTimestamp(overview: EnvironmentOverviewResponse) { | ||
| return overview.updated_at ? new Date(overview.updated_at).getTime() : 0 |
Contributor
There was a problem hiding this comment.
P3: The Last update sort disagrees with the row display when updated_at is missing: the row shows “just now,” but this comparator sorts it as 1970. Use the same Date.now() fallback as the display.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At libs/domains/environments/feature/src/lib/environments-table/environment-section/environment-section.tsx, line 196:
<comment>The Last update sort disagrees with the row display when `updated_at` is missing: the row shows “just now,” but this comparator sorts it as 1970. Use the same `Date.now()` fallback as the display.</comment>
<file context>
@@ -192,6 +192,70 @@ function lastOperationTimestamp(overview: EnvironmentOverviewResponse) {
}
+function lastUpdateTimestamp(overview: EnvironmentOverviewResponse) {
+ return overview.updated_at ? new Date(overview.updated_at).getTime() : 0
+}
+
</file context>
Suggested change
| return overview.updated_at ? new Date(overview.updated_at).getTime() : 0 | |
| return new Date(overview.updated_at ?? Date.now()).getTime() |
RemiBonnet
force-pushed
the
feat/environment-list-sortable-columns
branch
from
September 25, 2026 13:27
1b1e094 to
71f741f
Compare
Contributor
There was a problem hiding this comment.
1 existing issue remains and no new issues found across 2 files (changes from recent commits).
Confidence score: 4/5
- In
libs/domains/environments/feature/src/lib/environments-table/environment-section/environment-section.tsx, rows withoutupdated_atget a different fallback timestamp on each comparator call, which can make their sort order inconsistent; use a stable fallback value for the comparison.
Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.
Re-trigger cubic
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## staging #3020 +/- ##
===========================================
- Coverage 50.84% 49.17% -1.68%
===========================================
Files 1313 528 -785
Lines 28937 14358 -14579
Branches 8637 4499 -4138
===========================================
- Hits 14713 7060 -7653
+ Misses 11822 6151 -5671
+ Partials 2402 1147 -1255
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Allow sorting the environment list by clicking on column headers, with a click toggling the direction between ascending and descending. Sortable columns: Environment (name), Last operation, Cluster and Last update. This mirrors the sortable behavior already used on the service rows (arrow-down for ascending, arrow-up for descending) and exposes the current sort state via aria-sort for accessibility. Defaults are preserved: ephemeral environments still start sorted by the most recent operation, other sections keep the alphabetical order. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
RemiBonnet
force-pushed
the
feat/environment-list-sortable-columns
branch
from
September 25, 2026 14:28
71f741f to
1852c31
Compare
Contributor
There was a problem hiding this comment.
3 issues found across 5 reviewed files. 1 file intentionally excluded from review.
Confidence score: 3/5
- In
environment-section.tsx, the initially active sort is announced as unsorted until a header is clicked, so screen-reader users get incorrect sort status; pass the resolved default sort toSortableColumnHeaderor initialize equivalent state. - In
service-list.tsx, sortable service headers lackaria-sort, leaving screen-reader users unable to determine the sort direction. The spec checks icon classes rather than this accessibility contract, so assert the header’saria-sortinservice-list.spec.tsx.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="libs/domains/environments/feature/src/lib/environments-table/environment-section/environment-section.tsx">
<violation number="1" location="libs/domains/environments/feature/src/lib/environments-table/environment-section/environment-section.tsx:284">
P2: The default-sorted column is announced as unsorted until the user clicks a header. Pass the resolved default sort to `SortableColumnHeader` (or initialize equivalent state) so `aria-sort` reports the actual active column and direction.</violation>
</file>
<file name="libs/domains/services/feature/src/lib/service-list/service-list.tsx">
<violation number="1" location="libs/domains/services/feature/src/lib/service-list/service-list.tsx:252">
P2: This newly sortable service column has no `aria-sort` state in the shared header branch, so screen readers cannot tell whether it is unsorted, ascending, or descending. Add the current sort state to the `Table.ColumnHeaderCell`, including `none` when inactive.</violation>
</file>
<file name="libs/domains/services/feature/src/lib/service-list/service-list.spec.tsx">
<violation number="1" location="libs/domains/services/feature/src/lib/service-list/service-list.spec.tsx:455">
P3: This test asserts the FontAwesome glyph classes, which are an implementation detail, while the feature's advertised accessibility contract (header aria-sort) goes unverified. Assert the header's aria-sort attribute (ascending/descending/none) instead of, or in addition to, the icon classes so the test keeps checking meaning when the icon library or glyph names change.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
|
|
||
| // Ephemeral environments default to the most recent operation first, every other | ||
| // section keeps the historical alphabetical order. Clicking a header overrides this. | ||
| const [sort, setSort] = useState<EnvironmentSort | null>(null) |
Contributor
There was a problem hiding this comment.
P2: The default-sorted column is announced as unsorted until the user clicks a header. Pass the resolved default sort to SortableColumnHeader (or initialize equivalent state) so aria-sort reports the actual active column and direction.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At libs/domains/environments/feature/src/lib/environments-table/environment-section/environment-section.tsx, line 284:
<comment>The default-sorted column is announced as unsorted until the user clicks a header. Pass the resolved default sort to `SortableColumnHeader` (or initialize equivalent state) so `aria-sort` reports the actual active column and direction.</comment>
<file context>
@@ -280,32 +281,32 @@ export function EnvironmentSection({
- ? { column: 'last-operation', direction: 'desc' }
- : { column: 'name', direction: 'asc' }
- )
+ const [sort, setSort] = useState<EnvironmentSort | null>(null)
const handleSort = useCallback((column: EnvironmentSortColumn) => {
</file context>
| id: 'last_deployment', | ||
| header: 'Last operation', | ||
| enableColumnFilter: false, | ||
| enableSorting: true, |
Contributor
There was a problem hiding this comment.
P2: This newly sortable service column has no aria-sort state in the shared header branch, so screen readers cannot tell whether it is unsorted, ascending, or descending. Add the current sort state to the Table.ColumnHeaderCell, including none when inactive.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At libs/domains/services/feature/src/lib/service-list/service-list.tsx, line 252:
<comment>This newly sortable service column has no `aria-sort` state in the shared header branch, so screen readers cannot tell whether it is unsorted, ascending, or descending. Add the current sort state to the `Table.ColumnHeaderCell`, including `none` when inactive.</comment>
<file context>
@@ -240,19 +240,27 @@ export function ServiceList({ className, containerClassName, environment, ...pro
+ id: 'last_deployment',
+ header: 'Last operation',
+ enableColumnFilter: false,
+ enableSorting: true,
+ sortingFn: 'basic',
+ sortDescFirst: false,
</file context>
| .slice(1) | ||
| .map((row) => row.textContent?.match(/FRONT-END|back-end-A|CRONJOB|seed_script/)?.[0]) | ||
|
|
||
| expect(lastOperationHeader.querySelector('.fa-arrow-down, .fa-arrow-up')).not.toBeInTheDocument() |
Contributor
There was a problem hiding this comment.
P3: This test asserts the FontAwesome glyph classes, which are an implementation detail, while the feature's advertised accessibility contract (header aria-sort) goes unverified. Assert the header's aria-sort attribute (ascending/descending/none) instead of, or in addition to, the icon classes so the test keeps checking meaning when the icon library or glyph names change.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At libs/domains/services/feature/src/lib/service-list/service-list.spec.tsx, line 455:
<comment>This test asserts the FontAwesome glyph classes, which are an implementation detail, while the feature's advertised accessibility contract (header aria-sort) goes unverified. Assert the header's aria-sort attribute (ascending/descending/none) instead of, or in addition to, the icon classes so the test keeps checking meaning when the icon library or glyph names change.</comment>
<file context>
@@ -443,6 +443,31 @@ describe('ServiceList', () => {
+ .slice(1)
+ .map((row) => row.textContent?.match(/FRONT-END|back-end-A|CRONJOB|seed_script/)?.[0])
+
+ expect(lastOperationHeader.querySelector('.fa-arrow-down, .fa-arrow-up')).not.toBeInTheDocument()
+ expect(getServiceNames()).toEqual(['FRONT-END', 'back-end-A', 'CRONJOB', 'seed_script'])
+
</file context>
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
1 participant
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.
Summary
Issue:
Makes the environment list table sortable by clicking on the column headers, as requested. Each click on a sortable header toggles the sort direction between ascending and descending.
Sortable columns
updated_atImplementation details
EnvironmentSection(one per section) driving the existing customTablePrimitivestable.arrow-down(ascending) /arrow-up(descending) icons, so the behavior is consistent with the rest of the Console.aria-sorton the column headers for accessibility.Screenshots / Recordings
UI-only interaction change; header labels and layout are unchanged aside from the added sort arrows on click.
Testing
aria-sortstate (environment-section.spec.tsx)yarn test/yarn format/yarn lintPR Checklist
🤖 Generated with Claude Code
Summary by cubic
Makes the environment list table sortable by clicking on column headers, and enables the same sorting on the service list's Last operation column; each click toggles between ascending and descending.
updated_at); services sort by Last operation.aria-sort.updated_atvalues sort as if updated just now.last_deployment_datefrom deployment status so recent deployments order correctly.Written for commit 1852c31. Summary will update on new commits.