[Dream-76] Primerise Interface tab of Admin/Design page by bsatarnejad · Pull Request #24854 · opf/openproject

bsatarnejad · 2026-08-20T11:53:31Z

Ticket

https://community.openproject.org/wp/DREAM-76

What are you trying to accomplish?

Primerise interface tab in admin->design(interface)

Screenshots

github-actions · 2026-08-20T12:14:43Z

Warning

Flaky specs

rspec ./modules/gantt/spec/features/timeline/timeline_dates_spec.rb[1:2:1]
rspec ./spec/features/projects/lists/filters_spec.rb[1:6:1]

🤖 Ask Copilot to investigate

Copy the prompt below into a new comment on this PR to delegate the investigation to GitHub Copilot. It will look into the flakiness and open a separate pull request with you as reviewer.

@copilot The following spec(s) are flaky in CI (first seen on PR #24854, linked for reference only):

- `rspec ./modules/gantt/spec/features/timeline/timeline_dates_spec.rb[1:2:1]`
- `rspec ./spec/features/projects/lists/filters_spec.rb[1:6:1]`

Treat this as a standalone task, unrelated to PR #24854. Create a new branch from origin/dev and open a new pull request targeting dev — do not stack it on PR #24854 or reuse that branch.

Follow the playbook in docs/development/testing/handling-flaky-tests/README.md to find the root cause and fix the underlying race — do not skip, delete, or weaken the spec to make it pass; disabling is a last resort per the playbook, and only with a bug ticket. Verify the fix by running the spec(s) repeatedly (e.g. `script/bulk_run_rspec --run-count 10`).

If you cannot reproduce the flake or are not confident in a fix after reasonable investigation, do not fabricate a change or skip the spec to force CI green. Instead, leave the pull request in draft and document what you tried, the suspected cause, and any leads in its description, then assign @bsatarnejad to take over.

Once the fix is verified, title the PR after the spec(s) it fixes, and use the PR description to explain the root cause, how the change resolves it, and the before/after results. Label the PR `flaky-spec`, assign @bsatarnejad, and request a review from @bsatarnejad.
On every commit, set @bsatarnejad as the sole co-author with a `Co-authored-by:` trailer (use their GitHub no-reply email so it links to their account), so it is traceable who dispatched the fix.

github-actions · 2026-08-20T12:47:37Z

Warning

Flaky specs

rspec ./modules/backlogs/spec/features/inbox_column_spec.rb[1:7:1]
rspec ./spec/features/work_packages/table/switch_types_spec.rb[1:1:1]

🤖 Ask Copilot to investigate

Copy the prompt below into a new comment on this PR to delegate the investigation to GitHub Copilot. It will look into the flakiness and open a separate pull request with you as reviewer.

@copilot The following spec(s) are flaky in CI (first seen on PR #24854, linked for reference only):

- `rspec ./modules/backlogs/spec/features/inbox_column_spec.rb[1:7:1]`
- `rspec ./spec/features/work_packages/table/switch_types_spec.rb[1:1:1]`

Treat this as a standalone task, unrelated to PR #24854. Create a new branch from origin/dev and open a new pull request targeting dev — do not stack it on PR #24854 or reuse that branch.

Follow the playbook in docs/development/testing/handling-flaky-tests/README.md to find the root cause and fix the underlying race — do not skip, delete, or weaken the spec to make it pass; disabling is a last resort per the playbook, and only with a bug ticket. Verify the fix by running the spec(s) repeatedly (e.g. `script/bulk_run_rspec --run-count 10`).

If you cannot reproduce the flake or are not confident in a fix after reasonable investigation, do not fabricate a change or skip the spec to force CI green. Instead, leave the pull request in draft and document what you tried, the suspected cause, and any leads in its description, then assign @bsatarnejad to take over.

Once the fix is verified, title the PR after the spec(s) it fixes, and use the PR description to explain the root cause, how the change resolves it, and the before/after results. Label the PR `flaky-spec`, assign @bsatarnejad, and request a review from @bsatarnejad.
On every commit, set @bsatarnejad as the sole co-author with a `Co-authored-by:` trailer (use their GitHub no-reply email so it links to their account), so it is traceable who dispatched the fix.

github-actions · 2026-08-20T13:02:02Z

Caution

The Enterprise plan field is not set on the work package

Details:

Work package URL: https://community.openproject.org/wp/DREAM-76

Please make sure that:

The work package Enterprise plan field is set

Copilot

Pull request overview

This PR primerizes the Interface tab under Admin → Design, replacing the legacy form-based UI with Primer components (grouped color sections and per-color edit dialogs) and introducing an auto-submitting theme selector consistent across relevant tabs.

Changes:

Reworks the Interface tab UI into Primer headings + grouped BorderBox sections, with per-color edit dialogs.
Switches the theme selector to an auto-submit Primer::Alpha::Select and updates feature specs accordingly.
Adds translations and helper logic to support grouped colors and inherited/theme-based effective values.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
spec/features/custom_styles/tabs_navigation_spec.rb	Updates feature coverage for the new Primer UI, including dialog-based editing and auto-submit theme selection.
spec/controllers/custom_styles_controller_spec.rb	Adds controller-level coverage for updating themes while preserving the selected tab.
config/locales/en.yml	Adds labels/captions for color groups and per-color editing text.
app/views/custom_styles/show.html.erb	Replaces the old theme selector UI with a Primer Select wired to an auto-submit Stimulus controller.
app/views/custom_styles/_interface.html.erb	Primerizes the Interface tab layout and replaces the bulk form with per-color edit dialogs.
app/helpers/custom_styles_helper.rb	Introduces grouping and “effective color” helper logic to support the new UI rendering.

Suppressed comments (1)

spec/features/custom_styles/tabs_navigation_spec.rb:98

This example relies on opening and submitting a Primer dialog. In non-JS feature specs the dialog open/close behavior is not executed, which can make the test fail or become flaky. Add :js so the test runs with a JS driver.

    it "restores the inherited color by clearing an override" do

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

HDinger

The UI looks good to me 👍 I think there are some code improvements that need to be done before merging though

HDinger · 2026-08-24T12:04:34Z

+<%= render(
+      Primer::Beta::Heading.new(
+        tag: :h2,
+        font_size: 3,
+        font_weight: :normal,
+        mt: 4,
+        mb: 2,
+        pb: 2,
+        border: :bottom
+      )
+    ) do %>
+  <%= t(:label_interface_colors) %>
+<% end %>
+
+<%= render(Primer::Beta::Text.new(tag: :p, mb: 3)) do %>
+  <%= t(:label_interface_colors_description) %>
+<% end %>


I'd like to challenge the design here, and recommend to use a Primer::Beta::Subhead instead. It would give us a description slot which is how we usually display these additional texts. I am aware, that the designs show it differently but I would be willing to disucss that.

I agree, It may differ slightly from Figma, but it improves consistency and semantics.

HDinger · 2026-08-24T12:18:19Z

+      <% variable = design_color.variable %>
+      <% label = t("admin.custom_styles.colors.#{variable}") %>
+      <% instruction = t("admin.custom_styles.instructions.#{variable}", default: "") %>
+      <% hexcode = effective_design_color(design_color, current_theme: @current_theme) %>
+      <% form_id = "design-color-#{variable}-form" %>


It reads weird to me to have these things as variables. Given that this is an HTML file, I'd expect the things to be written inline directly. Is there a reason to extract them?

They’re reused for each row: the translations, color, and form ID each appear multiple times. Keeping them avoids repeated lookups and ensures dialog form IDs remain consistent.

HDinger · 2026-08-24T12:27:58Z

+    <%= form_tag(
+          update_design_themes_path,
+          method: :post,
+          data: {
+            controller: "auto-submit",
+            turbo_confirm: (t("admin.custom_styles.theme_warning") if @current_theme.blank?)
+          }
+        ) do %>
+      <%= render(
+            Primer::Alpha::Select.new(
+              name: "theme",
+              label: t("admin.custom_styles.color_theme"),
+              caption: t("admin.custom_styles.color_theme_caption"),
+              input_width: :large,
+              data: {
+                action: "auto-submit#submit",
+                test_selector: "color-theme-select"
+              }
+            )
+          ) do |select| %>
+        <% @theme_options.each do |option| %>
+          <% label, value, attributes = option.is_a?(Array) ? option : [option, option, {}] %>
+          <% select.option(
+               label:,
+               value:,
+               selected: attributes&.fetch(:selected, value == @current_theme),
+               disabled: attributes&.fetch(:disabled, false)
+             ) %>
+        <% end %>
+      <% end %>


Since this will be reused on the other tabs, I guess it makes sense to extract it into a component

Good point. I can extract it to a component to make the view easier to read. But it will be used only once in show.html.erb, because this view is shared by both Interface and Branding tabs.

github-actions · 2026-08-28T10:42:23Z

Warning

Flaky specs

rspec ./modules/backlogs/spec/features/inbox_column_spec.rb[1:7:1]
rspec ./modules/wikis/spec/features/admin/internal_provider_spec.rb[1:1]

🤖 Ask Copilot to investigate

Copy the prompt below into a new comment on this PR to delegate the investigation to GitHub Copilot. It will look into the flakiness and open a separate pull request with you as reviewer.

@copilot The following spec(s) are flaky in CI (first seen on PR #24854, linked for reference only):

- `rspec ./modules/backlogs/spec/features/inbox_column_spec.rb[1:7:1]`
- `rspec ./modules/wikis/spec/features/admin/internal_provider_spec.rb[1:1]`

Treat this as a standalone task, unrelated to PR #24854. Create a new branch from origin/dev and open a new pull request targeting dev — do not stack it on PR #24854 or reuse that branch.

Follow the playbook in docs/development/testing/handling-flaky-tests/README.md to find the root cause and fix the underlying race — do not skip, delete, or weaken the spec to make it pass; disabling is a last resort per the playbook, and only with a bug ticket. Verify the fix by running the spec(s) repeatedly (e.g. `script/bulk_run_rspec --run-count 10`).

If you cannot reproduce the flake or are not confident in a fix after reasonable investigation, do not fabricate a change or skip the spec to force CI green. Instead, leave the pull request in draft and document what you tried, the suspected cause, and any leads in its description, then assign @bsatarnejad to take over.

Once the fix is verified, title the PR after the spec(s) it fixes, and use the PR description to explain the root cause, how the change resolves it, and the before/after results. Label the PR `flaky-spec`, assign @bsatarnejad, and request a review from @bsatarnejad.
On every commit, set @bsatarnejad as the sole co-author with a `Co-authored-by:` trailer (use their GitHub no-reply email so it links to their account), so it is traceable who dispatched the fix.

github-actions · 2026-08-28T11:29:18Z

Warning

Flaky specs

rspec ./modules/gantt/spec/features/timeline/timeline_dates_spec.rb[1:2:1]

🤖 Ask Copilot to investigate

Copy the prompt below into a new comment on this PR to delegate the investigation to GitHub Copilot. It will look into the flakiness and open a separate pull request with you as reviewer.

@copilot The following spec(s) are flaky in CI (first seen on PR #24854, linked for reference only):

- `rspec ./modules/gantt/spec/features/timeline/timeline_dates_spec.rb[1:2:1]`

Treat this as a standalone task, unrelated to PR #24854. Create a new branch from origin/dev and open a new pull request targeting dev — do not stack it on PR #24854 or reuse that branch.

Follow the playbook in docs/development/testing/handling-flaky-tests/README.md to find the root cause and fix the underlying race — do not skip, delete, or weaken the spec to make it pass; disabling is a last resort per the playbook, and only with a bug ticket. Verify the fix by running the spec(s) repeatedly (e.g. `script/bulk_run_rspec --run-count 10`).

If you cannot reproduce the flake or are not confident in a fix after reasonable investigation, do not fabricate a change or skip the spec to force CI green. Instead, leave the pull request in draft and document what you tried, the suspected cause, and any leads in its description, then assign @bsatarnejad to take over.

Once the fix is verified, title the PR after the spec(s) it fixes, and use the PR description to explain the root cause, how the change resolves it, and the before/after results. Label the PR `flaky-spec`, assign @bsatarnejad, and request a review from @bsatarnejad.
On every commit, set @bsatarnejad as the sole co-author with a `Co-authored-by:` trailer (use their GitHub no-reply email so it links to their account), so it is traceable who dispatched the fix.

github-actions · 2026-08-28T13:43:43Z

Warning

Flaky specs

rspec ./spec/features/projects/create_spec.rb[1:12:3:2:2]
rspec ./spec/features/work_packages/details/inplace_editor/version_editor_spec.rb[1:1:2]

🤖 Ask Copilot to investigate

Copy the prompt below into a new comment on this PR to delegate the investigation to GitHub Copilot. It will look into the flakiness and open a separate pull request with you as reviewer.

@copilot The following spec(s) are flaky in CI (first seen on PR #24854, linked for reference only):

- `rspec ./spec/features/projects/create_spec.rb[1:12:3:2:2]`
- `rspec ./spec/features/work_packages/details/inplace_editor/version_editor_spec.rb[1:1:2]`

Treat this as a standalone task, unrelated to PR #24854. Create a new branch from origin/dev and open a new pull request targeting dev — do not stack it on PR #24854 or reuse that branch.

Follow the playbook in docs/development/testing/handling-flaky-tests/README.md to find the root cause and fix the underlying race — do not skip, delete, or weaken the spec to make it pass; disabling is a last resort per the playbook, and only with a bug ticket. Verify the fix by running the spec(s) repeatedly (e.g. `script/bulk_run_rspec --run-count 10`).

If you cannot reproduce the flake or are not confident in a fix after reasonable investigation, do not fabricate a change or skip the spec to force CI green. Instead, leave the pull request in draft and document what you tried, the suspected cause, and any leads in its description, then assign @bsatarnejad to take over.

Once the fix is verified, title the PR after the spec(s) it fixes, and use the PR description to explain the root cause, how the change resolves it, and the before/after results. Label the PR `flaky-spec`, assign @bsatarnejad, and request a review from @bsatarnejad.
On every commit, set @bsatarnejad as the sole co-author with a `Co-authored-by:` trailer (use their GitHub no-reply email so it links to their account), so it is traceable who dispatched the fix.

HDinger

LGTM 👍 👏

bsatarnejad self-assigned this Aug 20, 2026

bsatarnejad added feature needs review labels Aug 20, 2026

bsatarnejad marked this pull request as ready for review August 20, 2026 13:01

myabc requested review from myabc and a lite review from Copilot August 20, 2026 18:08

Copilot AI reviewed Aug 20, 2026

View reviewed changes

Comment thread spec/features/custom_styles/tabs_navigation_spec.rb Outdated

Copilot started reviewing on behalf of myabc August 20, 2026 21:31 View session

HDinger requested changes Aug 24, 2026

View reviewed changes

bsatarnejad added 7 commits August 28, 2026 11:27

Primerize interface tab

2cd778f

Primerize color theme selector

6842576

Update specs

6449db7

Add border bottom to interface color and remove title from dialog

3c628c3

Fix rubocop errors

485e433

Primer dialog interaction specs now use :js

c93d9da

Use border box list component

9d31abb

bsatarnejad force-pushed the dream-76-primerise-Interface-tab-of-admin-design-page branch from 20f3c32 to 9d31abb Compare August 28, 2026 09:27

bsatarnejad added 3 commits August 28, 2026 11:38

Use a Primer::Beta::Subhead instead of Heading

785d328

Use flex_layout and undo several nested flex boxes

2a947d0

Use Primer form dsl

39c3e56

Create a separate component for dialog

139ea00

Create a component for theme selector

e3f1919

HDinger approved these changes Sep 4, 2026

View reviewed changes

HDinger added this to the 17.9.x milestone Sep 4, 2026

Uh oh!

Conversation

bsatarnejad commented Aug 20, 2026

Ticket

What are you trying to accomplish?

Screenshots

Uh oh!

github-actions Bot commented Aug 20, 2026

Uh oh!

github-actions Bot commented Aug 20, 2026

Uh oh!

github-actions Bot commented Aug 20, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

HDinger left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

HDinger Aug 24, 2026

Choose a reason for hiding this comment

Uh oh!

bsatarnejad Aug 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HDinger Aug 24, 2026

Choose a reason for hiding this comment

Uh oh!

bsatarnejad Aug 28, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

HDinger Aug 24, 2026

Choose a reason for hiding this comment

Uh oh!

bsatarnejad Aug 28, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Aug 28, 2026

Uh oh!

github-actions Bot commented Aug 28, 2026

Uh oh!

github-actions Bot commented Aug 28, 2026

Uh oh!

HDinger left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

bsatarnejad Aug 28, 2026 •

edited

Loading