Skip to content

fix(settings): restore compact save action chip - #1186

Open
SalemOurabi wants to merge 3 commits into
4gray:masterfrom
SalemOurabi:settings-save-action-chip
Open

fix(settings): restore compact save action chip#1186
SalemOurabi wants to merge 3 commits into
4gray:masterfrom
SalemOurabi:settings-save-action-chip

Conversation

@SalemOurabi

Copy link
Copy Markdown
Contributor

What changed

  • restore the Settings save action as a compact floating chip
  • keep the chip aligned to the trailing edge with its original rounded border treatment
  • reserve enough scroll space so the chip does not cover the final settings row
  • add an Electron E2E regression test for the compact width and edge alignment

Root cause

The save action had been changed from a floating chip into a full-width sticky footer. That made its blur surface much larger than the action itself and removed the intended spacing from the bottom edge.

User impact

The save action remains sticky and easy to reach, but its background and blur are limited to a compact chip instead of spanning the entire Settings content width.

Validation

  • pnpm nx run electron-backend-e2e:e2e-ci--src/settings-layout.e2e.ts (1 passed on the isolated fix branch)
  • pnpm nx run electron-backend-e2e:e2e-ci--src/settings.e2e.ts (8 passed before branch extraction)
  • pnpm nx lint electron-backend-e2e (0 errors; existing warnings only)
  • git diff --check

Copy link
Copy Markdown
Contributor Author

@greptileai Please review this pull request, with particular attention to the compact Settings save-action chip, sticky positioning, blur surface sizing, content overlap, and Electron layout regressions.

@greptile-apps

greptile-apps Bot commented Jul 15, 2026

Copy link
Copy Markdown

Greptile Summary

This PR restores the settings save action from a full-width sticky footer back to a compact floating chip, and fixes a responsive layout bug where padding: 0 in the ≤1100px media query was zeroing out the bottom reservation that prevents the chip from covering the final settings row.

  • CSS variables restored: --settings-action-bar-offset returns to 24px and --settings-action-bar-space to 112px; the .action-bar rule regains margin-left: auto, border: 1px solid, and border-radius: 18px.
  • Responsive fix: The padding: 0 override inside @media (max-width: 1100px) is removed, preserving the padding-bottom that reserves scroll space at all viewport widths.
  • New E2E test: settings-layout.e2e.ts runs at 1000×760 px (below the 1100px breakpoint) and verifies compact width, trailing-edge alignment, position: sticky, bottom ≥ 24px, padding-bottom ≥ 112px, mid-scroll in-viewport behaviour, and full-scroll non-overlap with the final about section.

Confidence Score: 5/5

Safe to merge — the changes are a well-scoped CSS restoration with a direct E2E regression guard running at the exact breakpoint being fixed.

The three-file change restores two CSS variables, reverts the action-bar styling to the compact chip, and removes the padding: 0 override that was stripping the scroll reservation at ≤1100px. The new E2E test at 1000×760 px exercises compact width, sticky offset, padding-bottom reservation, mid-scroll visibility, and full-scroll non-overlap — covering all the properties this fix depends on. No logic paths, data flows, or existing functionality are at risk.

No files require special attention.

Important Files Changed

Filename Overview
apps/web/src/_settings-theme.scss Restores --settings-action-bar-offset to 24px and --settings-action-bar-space to 112px, both previously zeroed out for the full-width footer style.
apps/web/src/app/settings/settings.component.scss Restores chip styling on .action-bar and removes the padding: 0 override in the ≤1100px media query; dialog-mode override gains align-self: stretch and margin-left: 0 but no explicit border reset, so it now inherits the full all-around border from the base rule.
apps/electron-backend-e2e/src/settings-layout.e2e.ts New deterministic E2E test at 1000×760 px covering compact width, trailing alignment, sticky position, bottom offset, padding-bottom reservation, mid-scroll in-viewport check, and full-scroll non-overlap with the final settings section.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["--settings-action-bar-offset: 24px\n--settings-action-bar-space: 112px\n(_settings-theme.scss)"] --> B
    B[".settings-layout\npadding-bottom: calc(\n  --settings-action-bar-space +\n  --settings-safe-bottom\n)"]
    C["@media (max-width: 1100px)\n.settings-layout { max-width: 100% }\n✅ padding: 0 REMOVED"]
    B --> D{".settings-container\n.dialog-mode?"}
    C -.->|"previously zeroed padding-bottom\nnow removed"| B
    D -->|"Yes"| E[".settings-layout\npadding-bottom: 0\naction-bar: position static\nalign-self: stretch"]
    D -->|"No"| F[".action-bar\nposition: sticky\nbottom: 24px + safe-area\nmargin-left: auto\nborder-radius: 18px\nborder: 1px solid"]
    F --> G["Chip stays visible while scrolling\nFinal row never covered ✓"]
    E --> H["Full-width static bar in dialog ✓"]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A["--settings-action-bar-offset: 24px\n--settings-action-bar-space: 112px\n(_settings-theme.scss)"] --> B
    B[".settings-layout\npadding-bottom: calc(\n  --settings-action-bar-space +\n  --settings-safe-bottom\n)"]
    C["@media (max-width: 1100px)\n.settings-layout { max-width: 100% }\n✅ padding: 0 REMOVED"]
    B --> D{".settings-container\n.dialog-mode?"}
    C -.->|"previously zeroed padding-bottom\nnow removed"| B
    D -->|"Yes"| E[".settings-layout\npadding-bottom: 0\naction-bar: position static\nalign-self: stretch"]
    D -->|"No"| F[".action-bar\nposition: sticky\nbottom: 24px + safe-area\nmargin-left: auto\nborder-radius: 18px\nborder: 1px solid"]
    F --> G["Chip stays visible while scrolling\nFinal row never covered ✓"]
    E --> H["Full-width static bar in dialog ✓"]
Loading

Reviews (3): Last reviewed commit: "fix(settings): preserve sticky action sp..." | Re-trigger Greptile

Comment thread apps/electron-backend-e2e/src/settings-layout.e2e.ts
@SalemOurabi
SalemOurabi marked this pull request as ready for review July 15, 2026 03:11
@codecov-commenter

codecov-commenter commented Jul 15, 2026

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 59.02%. Comparing base (e91cbd5) to head (92dab54).
⚠️ Report is 1992 commits behind head on master.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

❗ There is a different number of reports uploaded between BASE (e91cbd5) and HEAD (92dab54). Click for more details.

HEAD has 4 uploads less than BASE
Flag BASE (e91cbd5) HEAD (92dab54)
4 0
Additional details and impacted files
@@             Coverage Diff             @@
##           master    #1186       +/-   ##
===========================================
- Coverage   71.05%   59.02%   -12.03%     
===========================================
  Files          40      593      +553     
  Lines         691    34271    +33580     
  Branches       87     7743     +7656     
===========================================
+ Hits          491    20230    +19739     
- Misses        176    11096    +10920     
- Partials       24     2945     +2921     
Flag Coverage Δ
unit 59.02% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@SalemOurabi

Copy link
Copy Markdown
Contributor Author

@greptileai please re-review the latest settings follow-up. The responsive layout now preserves sticky action spacing at <=1100px, with deterministic Electron E2E coverage at 1000x760.

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