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

Hide the abort button in a network subworkflow during installation #1195

Merged
merged 8 commits into from Apr 7, 2021

Conversation

dgdavid
Copy link
Member

@dgdavid dgdavid commented Apr 5, 2021

Problem

When user presses Abort button on Network settings page, it works same way as "Back" button and returns to the previous dialog. — https://bugzilla.suse.com/show_bug.cgi?id=1183586

As stated in the second comment, this happens

because the network subworkflow should not use [Abort] (...) It should be possible to abort only in the main workflow.

Solution

To use an installation argument (hide_abort_button) for setting when the abort button should be hide, since hiding it always during installation will break the installation UI consistence when the client is used in the main workflow.

Notes

As it can be seen in the proposed changes, the call to Yast::Wizard.HideAbortButton has been placed in src/include/network/lan/complex.rb #MainDialog method, since trying to place it somewhere before in the backtrace does not work (it worked for Yast::Wizard::DisableAbortButton but not for hiding it, which actually worked partially being hide and displayed again 😕).

For the record, the full path is

src/lib/network/clients/inst_lan.rb -> src/include/network/lan/wizards.rb#LanSequences -> src/include/network/lan/wizards.rb#MainSequence -> src/include/network/lan/complex.rb#MainDialog

Tests

  • Only tested manually using yupdate to apply changes in a SLE 15-SP3 Build 168.1

Screenshots

Patching the installer The client running in an installation sub-workflow, w/o the abort button
Patching the SUSE SLE installer via yupdate Network inst_lan client without the abort button

Related Trello card (internal link): https://trello.com/c/6M4weND5

As the inst_lan client is called from a sub-workflow, and abort is only
possible in the main workflow.
Copy link
Member Author

@dgdavid dgdavid left a comment

Choose a reason for hiding this comment

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

Should we sent this patch for SLE-15-SP3?

@dgdavid dgdavid requested a review from teclator April 5, 2021 15:53
@coveralls
Copy link

coveralls commented Apr 5, 2021

Coverage Status

Coverage decreased (-0.005%) to 79.608% when pulling f979d85 on bsc-1183586 into c7c776a on master.

@teclator
Copy link
Contributor

teclator commented Apr 5, 2021

The main problem I see is when it is called from the main workflow:

https://github.com/yast/skelcd-control-leanos/blob/master/control/control.leanos.xml#L565

Currently we are hiding it always during an installation or update.

BTW: I will try to check it deeper or later

Instead of hiding the abort button unconditionally, it does it according
to `disable_abort_button` inst argument. That's specially useful for
avoiding not having such button when the client is used in the main
installation workflow, keeping the installation UI consistence.
@dgdavid dgdavid changed the title Do not show the "Abort" button during installation Hide the abort button in a network installation subworkflow Apr 6, 2021
@dgdavid dgdavid changed the title Hide the abort button in a network installation subworkflow Hide the abort button in a network subworkflow during installation Apr 6, 2021
@dgdavid dgdavid requested review from ancorgs and removed request for imobachgs and lslezak April 6, 2021 16:28
@dgdavid
Copy link
Member Author

dgdavid commented Apr 6, 2021

Ey @teclator

Thanks a lot for your chat and hints today afternoon. I have updated the PR to use the new hide_abort_button installation argument for this client and created a new one in yast-registration, where the inst_client is called too.

Please, have a look again when you have time.

Co-authored-by: Knut Alejandro Anderssen González <kanderssen@suse.com>
Copy link
Contributor

@teclator teclator left a comment

Choose a reason for hiding this comment

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

LGTM

@dgdavid dgdavid requested a review from teclator April 7, 2021 12:32
Copy link
Contributor

@teclator teclator left a comment

Choose a reason for hiding this comment

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

LGTM

@dgdavid dgdavid merged commit 827f07d into master Apr 7, 2021
@dgdavid dgdavid deleted the bsc-1183586 branch April 7, 2021 14:57
@yast-bot
Copy link
Contributor

yast-bot commented Apr 7, 2021

✔️ Public Jenkins job #238 successfully finished
✔️ Created OBS submit request #883634

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

4 participants