Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move tool buttons to side pane #8703

Merged
merged 20 commits into from
Oct 25, 2024
Merged

Conversation

andreas-el
Copy link
Contributor

@andreas-el andreas-el commented Sep 16, 2024

Updated with library_add icon and renew plotter on subsequent clicks.

TGX
Screenshot 2024-10-25 at 12 21 07

macOS
Screenshot 2024-10-25 at 12 12 05

Update after comments
Added visual padding, reversed names in menu, added experiment type in menu description.

Screenshot 2024-10-23 at 08 42 57 1

Screenshot 2024-10-15 at 12 14 55

Issue
Resolves #8702

  • 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 tests pass locally (after every commit!)

When applicable

  • 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: Create Backport PR to latest release

TODO:

  • Fix layout / resizing, so pytest snapshots don't Error: Image dimensions did not match
  • Remove done-button completely, rely on signals
  • Fix notifier, so other widgets get updated
  • Look into how to share tools differently
  • Have text under icons in sidebar
  • Change toolbar order

@codecov-commenter
Copy link

codecov-commenter commented Sep 16, 2024

Codecov Report

Attention: Patch coverage is 98.94737% with 2 lines in your changes missing coverage. Please review.

Project coverage is 90.69%. Comparing base (d24eb13) to head (dd92fe1).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
src/ert/gui/main_window.py 98.71% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8703      +/-   ##
==========================================
- Coverage   90.82%   90.69%   -0.13%     
==========================================
  Files         352      349       -3     
  Lines       21759    21774      +15     
==========================================
- Hits        19762    19749      -13     
- Misses       1997     2025      +28     
Flag Coverage Δ
cli-tests 38.98% <0.00%> (+0.04%) ⬆️
gui-tests 71.70% <98.42%> (-0.87%) ⬇️
performance-tests 49.25% <21.05%> (-0.24%) ⬇️
unit-tests 79.54% <79.47%> (-0.09%) ⬇️

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

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

@andreas-el andreas-el force-pushed the side_panel_sidequest branch 2 times, most recently from fa37477 to e00765f Compare September 23, 2024 13:14
@andreas-el andreas-el force-pushed the side_panel_sidequest branch 5 times, most recently from f705166 to ef36952 Compare October 15, 2024 10:51
@andreas-el andreas-el force-pushed the side_panel_sidequest branch from fa4568b to 852e3a4 Compare October 17, 2024 07:54
@andreas-el andreas-el changed the title Side panel sidequest Move tool buttons to side pane Oct 18, 2024
@andreas-el andreas-el force-pushed the side_panel_sidequest branch from 0c0f562 to 5d2ed74 Compare October 18, 2024 13:23
@andreas-el andreas-el added the release-notes:user-impact Automatically categorise as breaking for people using CLI/GUI label Oct 18, 2024
@berland
Copy link
Contributor

berland commented Oct 21, 2024

Some observations:

  • In the "start simulation" pane, there should maybe be some more padding to the left of the dropdown menu
    image

  • In dropdown for "Simulation status" pane, add ensemble type to the timestamp for easier navigation. Order here is reversed compared to what we find under "Manage experiments".

The first icon in the sidepanel looks like the button to actually start the simulations, but it will not. Maybe we will learn to click the other play button..

@andreas-el
Copy link
Contributor Author

Some observations:

  • In the "start simulation" pane, there should maybe be some more padding to the left of the dropdown menu
    image
  • In dropdown for "Simulation status" pane, add ensemble type to the timestamp for easier navigation. Order here is reversed compared to what we find under "Manage experiments".

The first icon in the sidepanel looks like the button to actually start the simulations, but it will not. Maybe we will learn to click the other play button..

I agree we should replace the start button. Could any of these be an alternative? We are a bit limited by what EDS provides in the context of icons.

library_books
library_books

library_image
library_image

file
file

library_add
library_add

https://storybook.eds.equinor.com/?path=/docs/icons-preview--docs

@andreas-el andreas-el force-pushed the side_panel_sidequest branch from 79d7e07 to 3b8b305 Compare October 22, 2024 20:10
@sondreso
Copy link
Collaborator

I agree we should replace the start button. Could any of these be an alternative? We are a bit limited by what EDS provides in the context of icons.

library_books library_books

library_image library_image

file file

library_add library_add

https://storybook.eds.equinor.com/?path=/docs/icons-preview--docs

My vote would be on the library_add button

Copy link
Contributor

@eivindjahren eivindjahren left a comment

Choose a reason for hiding this comment

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

amazing! 👯

@andreas-el andreas-el merged commit 78890c9 into equinor:main Oct 25, 2024
56 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes:user-impact Automatically categorise as breaking for people using CLI/GUI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ert UI sidebar
5 participants