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

Show the dynamic buttons set in the main table of devices #1024

Merged
merged 5 commits into from Jan 17, 2020

Conversation

dgdavid
Copy link
Member

@dgdavid dgdavid commented Jan 16, 2020

Problem

The system overview page does not include the DeviceButtonSet at the bottom of the devices table. And... there is no arguable reason for it :)

Solution

To use those dynamic (and useful) buttons also on that page.

Side effects

To make the changes fully consistent with the rest of Partitioner pages, the second buttons row has been realigned to the right. Now, all pages follows the same pattern: contextual actions closest to the table and aligned to the left and general actions below but aligned to the right.

Tests

  • Added small unit tests to cover made changes
  • Tested manually (it works!)

Screenshots

Click to show/hide screenshots before realigning buttons
In a running system During Installation
in_running_system_before partitioner_installation_mode_before
ncurses_85x24_running_system_before

ncurses 85x24

ncurses_85x24_installation_before

ncurses 85x24

Click to show/hide screenshots after realigning buttons
In a running system During Installation
realigned_buttons_running_system realigned_buttons_installation
realigned_buttons_ncurses_85x24_running_system

ncurses 85x24

realigned_buttons_ncurses_85x24_installation

ncurses 85x24

@coveralls
Copy link

@coveralls coveralls commented Jan 16, 2020

Coverage Status

Coverage increased (+0.0003%) to 97.604% when pulling ac0465d on overview_with_dinamyc_buttons into d2fc4d9 on master.

@dgdavid dgdavid force-pushed the overview_with_dinamyc_buttons branch from 1bd990d to f89ca9d Compare Jan 17, 2020
package/yast2-storage-ng.changes Outdated Show resolved Hide resolved
@dgdavid dgdavid force-pushed the overview_with_dinamyc_buttons branch from f89ca9d to 11cf5c8 Compare Jan 17, 2020
# Page buttons
#
# @return [Array<Yast::UI::Term>]
def buttons
buttons = [rescan_devices_button]
buttons << import_mount_points_button if Yast::Mode.installation
buttons << HStretch()
Copy link
Member

@jreidinger jreidinger Jan 17, 2020

Choose a reason for hiding this comment

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

is there some agreement or decision to change how buttons look like? I see screenshot and not 100% sure it is better as it can have some original intention.

Copy link
Member Author

@dgdavid dgdavid Jan 17, 2020

Choose a reason for hiding this comment

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

I was discussing it for a while with @ancorgs locally. I proposed it (to have the discussion locally) in the daily call this morning, to make it faster and avoid uploading a bunch of useless screenshots.

Copy link
Contributor

@ancorgs ancorgs Jan 17, 2020

Choose a reason for hiding this comment

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

When we revamped the Partitioner interface in 15.1 we adopted a consistent buttons layout (agreed after many conversations among UX experts and developers) in all screens containing tables of devices except in two:

  • The NFS one because it's rendered by yast2-nfs-client
  • This one... for no particular reason. We simply did run out of time and energy to discuss it.

Since then, we have been considering the reasons for the old layout (two buttons at the left and another one at the right)... and we have found none. I guess it simply looked nicer to someone back then.

So we decided the time has come to make this screen consistent with all the others (well, except NFS, but that's another story).

And that consistency implies:

  • The first line of buttons is "contextual" (depends on the selected item) and aligned to the left
  • The second line are global buttons (they affect the whole list) and are aligned to the right

@dgdavid dgdavid force-pushed the overview_with_dinamyc_buttons branch from 06190c6 to fa5a31a Compare Jan 17, 2020
@dgdavid dgdavid force-pushed the overview_with_dinamyc_buttons branch from fa5a31a to 42475db Compare Jan 17, 2020
@dgdavid dgdavid force-pushed the overview_with_dinamyc_buttons branch from 42475db to ac0465d Compare Jan 17, 2020
@dgdavid dgdavid merged commit f806777 into master Jan 17, 2020
9 checks passed
@dgdavid dgdavid deleted the overview_with_dinamyc_buttons branch Jan 17, 2020
@yast-bot
Copy link

@yast-bot yast-bot commented Jan 17, 2020

✔️ Public Jenkins job #250 successfully finished
✔️ Created OBS submit request #765339

@yast-bot
Copy link

@yast-bot yast-bot commented Jan 17, 2020

✔️ Internal Jenkins job #73 successfully finished
✔️ Created IBS submit request #209627

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.

None yet

5 participants