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

Change text on the Reset All button in custom part. #2841

Merged

Conversation

VladimirSlavik
Copy link
Contributor

@VladimirSlavik VladimirSlavik commented Sep 9, 2020

Resolves: rhbz#1163701

Comments welcome. Previously the button said "Reset all".

Screenshot_vs_main_2020-09-22_13:59:12

@VladimirSlavik VladimirSlavik added the master Please, use the `f39` label instead. label Sep 9, 2020
@VladimirSlavik VladimirSlavik force-pushed the master-reset-all-new-name branch 2 times, most recently from 0fa5976 to a1092b9 Compare September 9, 2020 12:44
@poncovka poncovka added the notable change Important changes like API change, behavior change... label Sep 9, 2020
@AdamWill
Copy link
Contributor

AdamWill commented Sep 9, 2020

Yeah, I think this is definitely an improvement. 👍

Copy link
Contributor

@poncovka poncovka left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks!

Copy link
Contributor

@M4rtinK M4rtinK left a comment

Choose a reason for hiding this comment

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

Looks good to me as well, thanks - just a small wording improvement suggestion. :)

<property name="visible">True</property>
<property name="sensitive">False</property>
<property name="can_focus">True</property>
<property name="receives_default">True</property>
<property name="tooltip_text" translatable="yes" context="GUI|Custom Partitioning">Return to the state before any changes.</property>
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if "Roll back any changes" would be better ?

Copy link
Contributor Author

@VladimirSlavik VladimirSlavik Sep 10, 2020

Choose a reason for hiding this comment

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

Definitely! On a second though, that says the same as the button text?

@ShwetaNaresh
Copy link

How about any of the following?
Restore Default Settings | Cancel All | Discard Changes

@AdamWill
Copy link
Contributor

Definitely not "Restore Default Settings" because there are no default settings here. The operation we're describing is just cancelling all planned disk device changes in the installer. The existing state is "whatever state the disks are currently in". That's not something that can sensibly be described as a "default".

@ShwetaNaresh
Copy link

How about 'Restore Disk State'? 'Discard Changes' is more generic.

@AdamWill
Copy link
Contributor

That goes back to the initial problem of implying some kind of actual possibly-scary operation, I think. That's why we don't like "Reset All" - it can be ready as implying that clicking the button will actually do something possibly-scary to the disks. We need to try and make the new text avoid that as much as possible.

@VladimirSlavik
Copy link
Contributor Author

One more idea, Undo all changes.

Keep in mind the tooltip also needs some ideas. That can be longer.

@jkonecny12
Copy link
Member

One more idea, Undo all changes.

Maybe
Undo all planned actions.

Keep in mind the tooltip also needs some ideas. That can be longer.

What about
Return partitioning to the original state before user made any changes. This is not destructive operation.

@ShwetaNaresh
Copy link

IMO, with 'Undo All' we are discarding all changes, so in general term it could be labelled as 'Discard Changes'.
IMO, 'Undo' is generally considered to reset any previous action, and is represented more as an icon.

@AdamWill
Copy link
Contributor

Yeah, I quite like "discard" in this case.

@VladimirSlavik
Copy link
Contributor Author

VladimirSlavik commented Sep 22, 2020

Let's have the Discard variant... First comment edited for updated screenshot too.

@jkonecny12
Copy link
Member

Maybe I would remove the you in the tooltip. Something like:
...before any changes has been made...

Seems more formal to me.

@ShwetaNaresh
Copy link

ShwetaNaresh commented Sep 22, 2020 via email

@jkonecny12
Copy link
Member

Hi Jirka, Please can I know what tool tip are you referring to? Regards Shweta

On Tue, Sep 22, 2020, 6:16 PM Jiri Konecny @.***> wrote: Maybe I would remove the you in the tooltip. Something like: ...before any changes has been made... Seems more formal to me. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#2841 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMBFMTZPKYRGANI45PXMV6LSHDEULANCNFSM4RCEEFMA .

Tooltip on the screenshot here #2841 (comment) . Basically text which will show up when you hover mouse over the button.

Copy link
Contributor

@poncovka poncovka left a comment

Choose a reason for hiding this comment

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

It looks great Thanks!

@ShwetaNaresh
Copy link

ShwetaNaresh commented Sep 23, 2020 via email

@VladimirSlavik VladimirSlavik merged commit a3a9b57 into rhinstaller:master Sep 24, 2020
@VladimirSlavik VladimirSlavik deleted the master-reset-all-new-name branch September 24, 2020 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
master Please, use the `f39` label instead. notable change Important changes like API change, behavior change...
6 participants