Skip to content

fix(ngx): keep unavailable resources inspectable in declarative tables - #254

Open
anastazya wants to merge 2 commits into
openmfp:mainfrom
anastazya:fix/keep-unavailable-rows-inspectable
Open

fix(ngx): keep unavailable resources inspectable in declarative tables#254
anastazya wants to merge 2 commits into
openmfp:mainfrom
anastazya:fix/keep-unavailable-rows-inspectable

Conversation

@anastazya

@anastazya anastazya commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Problem

A resource with isAvailable === false is locked out of every interaction path in the declarative table stack — four independent layers: the row gets pointer-events: none, [interactive] is removed, onRowClick drops the emit, and resource-field disables every button including navigation (with a matching guard in the table-card handler).

A resource that is not ready is exactly the one a user needs to open to see why. Observed live on a Platform Mesh portal: with a readyCondition configured, a never-reconciled resource could not be opened at all — its warning icon offered no explanation and every path to the detail view was dead, while healthy resources navigated fine.

Fix

Keep the dimmed visual state, restore inspectability:

  • drop pointer-events: none; rows stay interactive
  • emit tableRowClicked unconditionally
  • disable only mutating buttons (update/delete) on unavailable resources; navigate and custom actions keep working

Specs: row-click contract flipped to the new behavior, new coverage for navigation buttons on unavailable resources. 592/592 ngx tests pass.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Navigation and custom actions now remain available when a resource is unavailable.
    • Update and delete actions remain disabled for unavailable resources.
    • Unavailable table rows can now be clicked and emit row-click events.
    • Row styling remains visually disabled while preserving interaction.

A resource with isAvailable === false was locked out of every interaction
path: the row got pointer-events: none, interactive was removed, onRowClick
dropped the emit, and resource-field disabled every button including
navigation. A resource that is not ready is exactly the one a user needs to
open to see why - with a readyCondition configured, failing resources became
completely uninspectable (observed live on the Platform Mesh portal: a
never-reconciled resource could not be opened at all, while its warning icon
offered no explanation).

- keep the dimmed styling but drop pointer-events: none and keep rows
  interactive
- emit tableRowClicked unconditionally
- disable only mutating buttons (update/delete) on unavailable resources;
  navigation and custom actions keep working
- flip the row-click spec to the new contract and cover navigation buttons
  on unavailable resources

All 592 ngx tests pass.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Theodor Ganescu <star.sg1@gmail.com>
@coderabbitai

coderabbitai Bot commented Aug 18, 2026

Copy link
Copy Markdown

Review Change Stack

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 587f35b0-8510-42e3-b797-769ef3479027

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: fa7cadd7-e2af-415e-97b0-2e8c8fb8c007

📥 Commits

Reviewing files that changed from the base of the PR and between bbacdd6 and 2f09433.

📒 Files selected for processing (7)
  • projects/ngx/declarative-ui/resource-field/resource-field.component.spec.ts
  • projects/ngx/declarative-ui/resource-field/resource-field.component.ts
  • projects/ngx/declarative-ui/table-card/declarative-table-card.component.ts
  • projects/ngx/declarative-ui/table/declarative-table/declarative-table.component.html
  • projects/ngx/declarative-ui/table/declarative-table/declarative-table.component.scss
  • projects/ngx/declarative-ui/table/declarative-table/declarative-table.component.spec.ts
  • projects/ngx/declarative-ui/table/declarative-table/declarative-table.component.ts
💤 Files with no reviewable changes (1)
  • projects/ngx/declarative-ui/table/declarative-table/declarative-table.component.scss

Included review availability: Your plan includes up to 1 review per rolling hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

Unavailable resources remain interactive for navigation and inspection. Update and delete actions remain blocked for unavailable resources. Resource field, table card, and table row tests reflect the updated behavior.

Changes

Unavailable resource interactions

Layer / File(s) Summary
Action availability handling
projects/ngx/declarative-ui/resource-field/resource-field.component.ts, projects/ngx/declarative-ui/table-card/declarative-table-card.component.ts, projects/ngx/declarative-ui/resource-field/resource-field.component.spec.ts
ResourceField and DeclarativeTableCard restrict unavailable-resource blocking to update and delete actions. Navigation actions remain enabled and emit events.
Unavailable row interaction
projects/ngx/declarative-ui/table/declarative-table/declarative-table.component.html, projects/ngx/declarative-ui/table/declarative-table/declarative-table.component.ts, projects/ngx/declarative-ui/table/declarative-table/declarative-table.component.spec.ts
Unavailable rows remain interactive and emit tableRowClicked with the resource. Tests now expect one emitted event.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 2f094

Unavailable resources remain inspectable while mutating actions stay disabled; no actionable merge-blocking risk remains after normal checks and review.

Possibly related PRs

Suggested reviewers: sobyt483

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: restoring inspectability for unavailable resources in declarative tables.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@gkrajniak

Copy link
Copy Markdown
Member

Here, we have the possibility to add the status reay or read whatever to indicate the satus or issue

Screenshot 2026-08-18 at 13 07 26

if navigating to the resource would give us more info, would that not be a case as well in the table view, I don't see anything that can give us more navigating in that scenaro to the resource, maybe some example or a screenshot what I am missing here?

@anastazya

Copy link
Copy Markdown
Contributor Author

Concrete case, from the Platform Mesh portal on our dev landscape this morning (Telekom OSC provider, which sets a readyCondition): a ShootClaim sat not-ready because no controller had reconciled it. The table showed the alert icon with accessible-name "Resource is not ready" and nothing else — and with the row locked there was no way into the detail page, which is where the conditions/messages render and where Edit/Delete live. Worse: when a resource is not-ready because its spec is wrong (bad region, bad version), the fix is Edit on the detail page — but the row is disabled precisely because the resource is broken, so the UI can never reach its own fix. The only escape was kubectl.

A status/reason column in the table (your screenshot) definitely helps visibility, and we're adding one to our fragment once the status contract is settled. But it's per-fragment config and one line — multi-condition detail and the remediation actions still live behind navigation. That's the case for keeping not-ready rows clickable and only blocking the mutating buttons.

@anastazya
anastazya marked this pull request as ready for review August 18, 2026 11:15
@anastazya
anastazya requested review from a team as code owners August 18, 2026 11:15
@gkrajniak

Copy link
Copy Markdown
Member

Great thanks for the insightfull description, now I see the breader picture here, and I will think analyze the proposed solution and how differently we can handle that situation as well.

@gkrajniak

Copy link
Copy Markdown
Member

the thing is that this change completly oboletes the idea of the resourcec.isAvailable flag

the last point:

  • disable only mutating buttons (update/delete) on unavailable resources; navigate and custom actions keep working
    is irrelevant if I navigate to a resource detail page and from there I am able to execute the updata or delete action, then why not in the table

so I am thinking how to address this differently maybe with a flag in the contentconfiguration

export interface ResourceDefinition {
  apiGroup?: string;
  version: string;
  entityCollection: string;
  entity: string;
  name?: string;
  scope?: KubernetesScope;
  namespace?: string;
  readyCondition?: PlatformMeshFieldDefinition;
  ui?: UIDefinition;
  permissionsDefinition?: PermissionsDefinition;
}

avilableWhenNotReady?: boolean
and afterwards the change will be only in pm/p-ui-lib ResourceService while setting the isAvaiable flag

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants