Skip to content

Fix/sonar issues and metrics bar - #4494

Merged
ChristianHuehn merged 3 commits into
mainfrom
fix/sonar-issues-and-metrics-bar
Jun 17, 2026
Merged

Fix/sonar issues and metrics bar#4494
ChristianHuehn merged 3 commits into
mainfrom
fix/sonar-issues-and-metrics-bar

Conversation

@ChristianHuehn

@ChristianHuehn ChristianHuehn commented Jun 17, 2026

Copy link
Copy Markdown
Collaborator

{Meaningful title}

Please read the CONTRIBUTING.md before opening a PR.

Closes: #

Description

Descriptive pull request text, answering:

  • What problem/issue are you fixing?
  • What does this PR implement and how?

Definition of Done

A PR is only ready for merge once all the following acceptance criteria are fulfilled:

  • Changes have been manually tested
  • All TODOs related to this PR have been closed
  • There are automated tests for newly written code and bug fixes
  • All bugs discovered while working on this PR have been submitted as issues (if not already an open issue)
  • Documentation (GH-pages, analysis/visualization READMEs, parser READMEs, --help, etc.) has been updated (almost always necessary except for bug fixes)
  • CHANGELOG.md has been updated

Screenshots or gifs

Summary by CodeRabbit

  • Accessibility
    • Added required accessible labels to slider/number inputs and search fields, improving screen reader support across metrics controls and popovers.
  • Bug Fixes
    • Improved reliability of dropdown outside-click handling with safer target type checks.
  • Style
    • Updated metrics bar positioning to improve consistent alignment across the UI.
  • Tests
    • Enhanced MetricsBar Playwright coverage by collapsing the explorer before interacting with metrics dropdowns.

christian-huehn-mw and others added 2 commits June 17, 2026 07:36
Mark never-reassigned members as readonly, drop unnecessary Node type
assertions in favor of instanceof guards, and add accessible names to
metrics-bar inputs and the metric search label.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Center the bottom metrics bar on the viewport instead of between the
sidebars, so resizing the explorer no longer shifts it. Remove the now
unused --cc-explorer-width / --cc-inspector-occupied-width plumbing.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 17, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b5d521a2-77d7-4375-8cf0-c0f0153dfb84

📥 Commits

Reviewing files that changed from the base of the PR and between b89a4f8 and 15dde41.

📒 Files selected for processing (2)
  • visualization/app/codeCharta/features/metricsBar/components/metricsBar/metricsBar.e2e.ts
  • visualization/app/playwright.helper.ts
✅ Files skipped from review due to trivial changes (1)
  • visualization/app/playwright.helper.ts

📝 Walkthrough

Walkthrough

Three main feature areas and one test support change: (1) SidebarExplorerComponent and SidebarInspectorComponent remove effect/OnDestroy-based publishing of --cc-explorer-width and --cc-inspector-occupied-width CSS variables; MetricsBarComponent replaces the dependent dynamic inline style bindings with static Tailwind classes. (2) SliderNumberInputComponent gains a required ariaLabel input propagated via [attr.aria-label] to both inputs; all consumer popovers supply the label. (3) Minor code quality: readonly modifiers added to constructor parameters/fields; event.target instanceof Node guards replace cast-based containment checks. (4) E2E test for metrics bar now collapses the explorer sidebar before interaction.

Changes

Remove CSS Variable Layout, Update MetricsBar Positioning

Layer / File(s) Summary
Remove CSS variable publishing from sidebar components
visualization/app/codeCharta/features/sidebarExplorer/components/sidebarExplorer/sidebarExplorer.component.ts, visualization/app/codeCharta/features/sidebarInspector/components/sidebarInspector/sidebarInspector.component.ts
SidebarExplorerComponent removes the publishOccupiedWidth effect and ngOnDestroy cleanup for --cc-explorer-width. SidebarInspectorComponent drops its effect/OnDestroy implementation for --cc-inspector-occupied-width and no longer implements OnDestroy.
MetricsBar host: static Tailwind positioning
visualization/app/codeCharta/features/metricsBar/components/metricsBar/metricsBar.component.ts
Host class gains left-0, right-0, and mx-auto, replacing inline style bindings that previously read --cc-explorer-width and --cc-inspector-occupied-width.

Add ariaLabel to SliderNumberInput and Consumer Templates

Layer / File(s) Summary
SliderNumberInput ariaLabel input, template, and test
visualization/app/codeCharta/features/metricsBar/components/sliderNumberInput/sliderNumberInput.component.ts, .../sliderNumberInput.component.html, .../sliderNumberInput.component.spec.ts
Adds ariaLabel = input.required<string>(), binds [attr.aria-label]="ariaLabel()" on both the range and number inputs, and updates the spec setup() to pass ariaLabel: "Test value".
Propagate ariaLabel to all consumer templates
visualization/app/codeCharta/features/metricsBar/components/areaSettingsPopover/areaSettingsPopover.component.html, .../heightSettingsPopover/heightSettingsPopover.component.html, .../edgeSettingsPopover/edgeSettingsPopover.component.html, .../colorSettingsPopover/metricColorRangeSlider.component.html, .../metricSelectPopover/metricSelectPopover.component.html
Area and height popovers add ariaLabel to their cc-slider-number-input elements; edge popover adds ariaLabel to its two sliders; color range slider adds aria-label to its two raw number inputs; metric select popover adds aria-label to the search label.

readonly Modifiers and instanceof Node Guards

Layer / File(s) Summary
readonly fields and instanceof Node containment guards
visualization/app/codeCharta/features/globalSettings/.../confirmResetMapDialog.component.ts, visualization/app/codeCharta/features/navBar/services/fileSelectionMode.service.ts, .../deltaSelector/deltaSelector.component.ts, .../explorerSortControl/explorerSortControl.component.ts
Constructor parameters and urlUtils/subscription fields gain private readonly. DeltaSelectorComponent and ExplorerSortControlComponent now guard event.target instanceof Node before calling contains().

E2E Test Support for Metrics Bar Interaction

Layer / File(s) Summary
E2E test and Playwright helper updates
visualization/app/codeCharta/features/metricsBar/components/metricsBar/metricsBar.e2e.ts, visualization/app/playwright.helper.ts
MetricsBar e2e test imports and calls collapseExplorer(page) before running tests to expose the left-most area segment. New collapseExplorer helper in playwright.helper.ts clicks the explorer-collapse-button test ID.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • MaibornWolff/codecharta#4485: Introduced the SidebarInspectorComponent refactor that added the effect/OnDestroy-based --cc-inspector-occupied-width CSS variable publishing that this PR removes.
  • MaibornWolff/codecharta#4493: Directly overlaps with this PR's removal of effect-driven --cc-explorer-width publishing in SidebarExplorerComponent.

Poem

🐇 Hop, hop! No more CSS vars to track,
The sidebar widths have packed their bags,
Tailwind classes hold the left and right in place,
While aria-labels grant each slider its grace.
readonly fields stand firm—no reassign!
And instanceof Node keeps the clicks in line. 🌸

🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (2 warnings, 1 inconclusive)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is entirely unmodified boilerplate template with empty sections, unchecked checklist items, and no substantive information about the actual changes or fixes. Fill in all required sections: provide a meaningful title, explain what problems are being fixed, describe implementation details, complete the Definition of Done checklist, and add relevant screenshots if applicable.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title is vague and generic, using non-descriptive terms 'Fix/sonar issues' that don't clearly convey what specific changes were made. Replace with a more specific title that clearly describes the main change, e.g., 'Add aria-label accessibility attributes and fix metrics bar e2e tests' or 'Improve accessibility labels and resolve metrics bar positioning issues'.
✅ Passed checks (2 passed)
Check name Status Explanation
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/sonar-issues-and-metrics-bar

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 and usage tips.

The metrics bar is centered on the viewport and intentionally sits behind
the open explorer, so its left-most area segment was unreachable and the
area-metric e2e tests timed out. Collapse the explorer first to expose it.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@sonarqubecloud

Copy link
Copy Markdown

@ChristianHuehn
ChristianHuehn merged commit 8c29a89 into main Jun 17, 2026
8 checks passed
@ChristianHuehn
ChristianHuehn deleted the fix/sonar-issues-and-metrics-bar branch June 17, 2026 07:21
@coderabbitai coderabbitai Bot mentioned this pull request Jun 24, 2026
6 tasks
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