Skip to content

Move style customization to the sidebar - #14025

Closed
MagnusSletten wants to merge 5 commits into
equinor:mainfrom
MagnusSletten:move-style-sidebar
Closed

Move style customization to the sidebar#14025
MagnusSletten wants to merge 5 commits into
equinor:mainfrom
MagnusSletten:move-style-sidebar

Conversation

@MagnusSletten

@MagnusSletten MagnusSletten commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Issue
Resolves #14021

Approach

Moved the style options from the Customize dialog to a collapsible section in the sidebar. Style selections are now shared across keys rather than configured per key, so the chosen styling remains consistent when switching keys. Added "compact" mode to StyleEdits to make them narrower for the sidebar.

image image image
  • PR title captures the intent of the changes, and is fitting for release notes.
  • Added appropriate release note label
  • Commit history is consistent and clean, in line with the contribution guidelines.
  • Make sure unit tests pass locally after every commit (git rebase -i main --exec 'just rapid-tests')

When applicable

  • When screenshots are changed: Review screenshot-PR in ert-testdata,
    merge screenshot-PR in ert-testdata before merging this PR.
  • When there are user facing changes: Updated documentation
  • New behavior or changes to existing untested code: Ensured that unit tests are added (See Ground Rules).
  • Large PR: Prepare changes in small commits for more convenient review
  • Bug fix: Add regression test for the bug
  • Bug fix: Add backport label to latest release (format: 'backport release-branch-name')

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Moves plot style customization controls out of the “Customize plot” dialog and into the plot window sidebar, by introducing a dedicated sidebar “Style options” panel and a compact StyleChooser presentation.

Changes:

  • Added a new sidebar StyleOptions widget (with reset + availability toggling) and wired it into PlotWindow.
  • Updated StyleChooser to support a compact rendering and emit a styleChanged signal.
  • Removed the old style customization view/tab and updated unit/UI tests accordingly.

Reviewed changes

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

Show a summary per file
File Description
src/ert/gui/plotting/widgets/plot_controls/style_options.py New sidebar panel for editing default/history/observation styles.
src/ert/gui/plotting/plot_window.py Instantiates/inserts the style panel into the sidebar and applies selected styles when plotting.
src/ert/gui/plotting/widgets/plot_controls/__init__.py Exports StyleOptions from plot controls.
src/ert/gui/plotting/customization_dialog/style_chooser.py Adds compact mode, compact marker symbols, and a styleChanged signal.
src/ert/gui/plotting/customization_dialog/customize_plot_dialog.py Removes the “Style” tab from the customization dialog.
src/ert/gui/plotting/customization_dialog/style_customization_view.py Removed obsolete customization view.
tests/ert/unit_tests/gui/plotting/widgets/test_style_options.py Adds unit coverage for the new sidebar style controls and compact chooser behavior.
tests/ert/ui_tests/gui/test_plot_customization.py Updates dialog logging assertions after removing the style tab.

Comment thread src/ert/gui/plotting/customization_dialog/style_chooser.py
@ertomatic

Copy link
Copy Markdown
Collaborator

Screenshots differ from baselines. A baseline update PR has been prepared: equinor/ert-testdata#64

@codecov-commenter

codecov-commenter commented Jul 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.52%. Comparing base (77e0dbb) to head (40aa35d).

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #14025      +/-   ##
==========================================
+ Coverage   91.48%   91.52%   +0.03%     
==========================================
  Files         487      488       +1     
  Lines       34086    34229     +143     
==========================================
+ Hits        31184    31327     +143     
  Misses       2902     2902              
Flag Coverage Δ
cli-tests 35.95% <0.00%> (-0.16%) ⬇️
fuzz 43.55% <22.51%> (-0.11%) ⬇️
gui-tests 58.83% <93.71%> (+0.17%) ⬆️
performance-and-unit-tests 80.05% <100.00%> (+0.02%) ⬆️
test 45.85% <22.51%> (-0.12%) ⬇️

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

Files with missing lines Coverage Δ
...ting/customization_dialog/customize_plot_dialog.py 80.88% <ø> (-0.17%) ⬇️
...gui/plotting/customization_dialog/style_chooser.py 97.93% <100.00%> (+2.88%) ⬆️
src/ert/gui/plotting/plot_window.py 79.79% <100.00%> (+0.42%) ⬆️
...ert/gui/plotting/widgets/plot_controls/__init__.py 100.00% <100.00%> (ø)
...t/gui/plotting/widgets/plot_controls/style_edit.py 100.00% <100.00%> (ø)
...ui/plotting/widgets/plot_controls/style_options.py 100.00% <100.00%> (ø)

... and 5 files with indirect coverage changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comment thread src/ert/gui/plotting/customization_dialog/style_chooser.py Outdated
ertomatic pushed a commit to equinor/ert-testdata that referenced this pull request Jul 24, 2026
@MagnusSletten
MagnusSletten requested a review from Copilot July 24, 2026 12:16

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

src/ert/gui/plotting/plot_window.py:590

  • The sidebar style selections are only copied into the PlotConfig when selected_tab == ENSEMBLE. However, the “Statistics” plot also uses PlotConfig.history_style()/observations_style() (plot_history/plotObservations), so style changes won’t be reflected there. Apply the StyleOptions styles regardless of the currently selected tab (or at least for both Ensemble and Statistics).
            if selected_tab == ENSEMBLE:
                plot_config.set_default_style(self._style_options.get_default_style())

            if not self.is_everest:

Comment thread src/ert/gui/plotting/customization_dialog/style_chooser.py
Comment thread tests/ert/ui_tests/gui/test_plot_customization.py Outdated
Comment thread src/ert/gui/plotting/plot_window.py
ertomatic pushed a commit to equinor/ert-testdata that referenced this pull request Jul 24, 2026
ertomatic pushed a commit to equinor/ert-testdata that referenced this pull request Jul 24, 2026
ertomatic pushed a commit to equinor/ert-testdata that referenced this pull request Jul 24, 2026
@MagnusSletten
MagnusSletten requested a review from eilskra July 24, 2026 13:35
@MagnusSletten MagnusSletten changed the title Move style sidebar Move style customization to the sidebar Jul 24, 2026
ertomatic pushed a commit to equinor/ert-testdata that referenced this pull request Jul 24, 2026
@MagnusSletten
MagnusSletten marked this pull request as ready for review July 24, 2026 14:04
ertomatic pushed a commit to equinor/ert-testdata that referenced this pull request Jul 24, 2026
@codspeed-hq

codspeed-hq Bot commented Jul 24, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 36 untouched benchmarks


Comparing MagnusSletten:move-style-sidebar (a53fc9c) with main (77e0dbb)

Open in CodSpeed

ertomatic pushed a commit to equinor/ert-testdata that referenced this pull request Jul 26, 2026
Introduce a reusable style edit widget that adapts StyleChooser for
compact use in the plot sidebar.

It provides the shared layout and style handling needed by both
ensemble and statistics controls, avoiding separate implementations
for each sidebar section.
ertomatic pushed a commit to equinor/ert-testdata that referenced this pull request Jul 26, 2026
@MagnusSletten
MagnusSletten requested a review from Copilot July 27, 2026 07:38
ertomatic pushed a commit to equinor/ert-testdata that referenced this pull request Jul 27, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comments suppressed due to low confidence (1)

src/ert/gui/plotting/customization_dialog/style_chooser.py:253

  • setStyle() updates the child widgets via setCurrentIndex() / setValue(), which emits their signals. That can (1) emit styleChanged and (2) consume log_plot_option_usage_once telemetry hooks, making resets/programmatic updates look like user actions and preventing later user interactions from being logged. Block child widget signals while applying a style programmatically.
    @override
    def setStyle(self, style: PlotStyle) -> None:  # type: ignore
        self._style.copy_style_from(style)
        self._update_line_style_and_marker(
            style.line_style, style.marker, style.width, style.size
        )

Comment on lines +235 to 239
line_style = self.line_chooser.currentData()
marker_style = self.marker_chooser.currentData()

line_style: str = self.line_chooser.itemData(self.line_chooser.currentIndex())
marker_style: str = self.marker_chooser.itemData(
self.marker_chooser.currentIndex()
)
self.marker_chooser.setEnabled(line_style != STYLE_AREA[1])
thickness = float(self.thickness_spinner.value())
@eilskra

eilskra commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Could this be simplified? Could we extend either PlotWidget, PlotConfig or PlotContext instead of adding additional files? If we could simplify the way we update linestyles etc, we could more easy implement style selection later

@MagnusSletten

Copy link
Copy Markdown
Contributor Author

Could this be simplified? Could we extend either PlotWidget, PlotConfig or PlotContext instead of adding additional files? If we could simplify the way we update linestyles etc, we could more easy implement style selection later

Yes, I think it could be simplifed quite a bit if we ditch even more of the legacy code than already done here. With the current PR changes what we are moving away from is the complications around per-key plot configuration which is good here because that added complexity did not quite make sense.

But what we could also do is to simplify everything around the plotstyle section itself. I can take a look at this.

@eilskra

eilskra commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

To be honest, due to the very limited use of the original implementation, I think we could remove the legacy code altogether and opt for a from-scratch implementation instead.

We know the functionality wanted (centralize being able change line/marker style and size). Might take a little bit longer, but I think the end result will be much better.

At least worth looking into, let me know what you think 👍

@MagnusSletten

MagnusSletten commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

To be honest, due to the very limited use of the original implementation, I think we could remove the legacy code altogether and opt for a from-scratch implementation instead.

We know the functionality wanted (centralize being able change line/marker style and size). Might take a little bit longer, but I think the end result will be much better.

At least worth looking into, let me know what you think 👍

I think it's definitely worth doing sooner rather than later, then we can avoid a second redesign going over this exact code. Let's do it here 👍

Closing this PR for less clutter.

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.

Remove line style customization

5 participants