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

Consistency for Edit/Save button labels and styles #7905

Merged
merged 5 commits into from Oct 10, 2018

Conversation

Projects
None yet
7 participants
@Soean
Member

Soean commented Jul 11, 2018

Description

Fixes #6469
Old PR: #7593 (messed up after rebase)

1.

  • Renames permalink-save button from OK to Save

Before:
bildschirmfoto 2018-06-27 um 23 22 11
After:
bildschirmfoto 2018-06-27 um 23 18 58

2.

  • Removes the Cancel button from Shared Blocks title edit
  • Removes primary button style from Save button
  • Cancels edit, if user clicks outside.

Before:
bildschirmfoto 2018-06-27 um 23 22 45
After:
bildschirmfoto 2018-06-27 um 23 19 22

@Soean

This comment has been minimized.

Show comment
Hide comment
@Soean

Soean Jul 11, 2018

Member

@noisysocks in the old PR:

But I'm worried that this change makes it easy for a user to lose their changes to a shared block. Right now, if a user is editing their shared block, and then clicks away to e.g. copy some text, their changes remain on screen. With this patch, if a user clicks away, any changes are lost and there is no way to undo the action.

Member

Soean commented Jul 11, 2018

@noisysocks in the old PR:

But I'm worried that this change makes it easy for a user to lose their changes to a shared block. Right now, if a user is editing their shared block, and then clicks away to e.g. copy some text, their changes remain on screen. With this patch, if a user clicks away, any changes are lost and there is no way to undo the action.

@Soean Soean requested review from karmatosed, jasmussen and noisysocks Jul 11, 2018

@ZebulanStanphill

This comment has been minimized.

Show comment
Hide comment
@ZebulanStanphill

ZebulanStanphill Jul 12, 2018

Contributor

Is Save really correct for the permalink editor? The permalink is not actually saved until an auto or manual full post save happens.

I think the button should be Done in order to be more descriptive of what it actually does. Otherwise a user may think that they do not need to use the main Publish/Update button in order to save their changes, and will be confused when they try to leave the page and their browser tells them that there are unsaved changes, or they may want to not save certain changes and leave the page but do not realize that the permalink change is one of them.

Contributor

ZebulanStanphill commented Jul 12, 2018

Is Save really correct for the permalink editor? The permalink is not actually saved until an auto or manual full post save happens.

I think the button should be Done in order to be more descriptive of what it actually does. Otherwise a user may think that they do not need to use the main Publish/Update button in order to save their changes, and will be confused when they try to leave the page and their browser tells them that there are unsaved changes, or they may want to not save certain changes and leave the page but do not realize that the permalink change is one of them.

@karmatosed

We do have to be cautious to see if people are ok about just having 'save', however it is a pattern many applications use. Let's get this in and take from there observing.

@noisysocks

This comment has been minimized.

Show comment
Hide comment
@noisysocks

noisysocks Jul 16, 2018

Member

losing-changes

I'm still very concerned about this making it easy for a user to accidentally lose their changes. I'd prefer that we keep the Cancel button. Any thoughts, @karmatosed?

Member

noisysocks commented Jul 16, 2018

losing-changes

I'm still very concerned about this making it easy for a user to accidentally lose their changes. I'd prefer that we keep the Cancel button. Any thoughts, @karmatosed?

@karmatosed

This comment has been minimized.

Show comment
Hide comment
@karmatosed

karmatosed Jul 16, 2018

Member

@noisysocks whilst I understand that concern I have to say that people tend to expect just one button. They don't expect 'cancel' and 'save'. How about this do you have any examples of actions like this where cancel is used? I'd love to pan out a bit in consideration.

Member

karmatosed commented Jul 16, 2018

@noisysocks whilst I understand that concern I have to say that people tend to expect just one button. They don't expect 'cancel' and 'save'. How about this do you have any examples of actions like this where cancel is used? I'd love to pan out a bit in consideration.

@noisysocks

This comment has been minimized.

Show comment
Hide comment
@noisysocks

noisysocks Jul 17, 2018

Member

How about this do you have any examples of actions like this where cancel is used?

The only thing that comes to my mind is when you close a new document in e.g. TextEdit and it asks you what you want to do with your unsaved changes.


How about we simply keep the unsaved changes when the block is unselected?

save

Pro: No longer easy to accidentally lose your unsaved changes.
Con: Easier to forget to click Save, unable to discard unsaved changes and see what is currently published.

Curious if @jasmussen has any ideas here—from memory, the Edit / Cancel / Save flow originally came from a conversation that we had last year 🙂

Member

noisysocks commented Jul 17, 2018

How about this do you have any examples of actions like this where cancel is used?

The only thing that comes to my mind is when you close a new document in e.g. TextEdit and it asks you what you want to do with your unsaved changes.


How about we simply keep the unsaved changes when the block is unselected?

save

Pro: No longer easy to accidentally lose your unsaved changes.
Con: Easier to forget to click Save, unable to discard unsaved changes and see what is currently published.

Curious if @jasmussen has any ideas here—from memory, the Edit / Cancel / Save flow originally came from a conversation that we had last year 🙂

@karmatosed

This comment has been minimized.

Show comment
Hide comment
@karmatosed

karmatosed Jul 24, 2018

Member

How about we simply keep the unsaved changes when the block is unselected?

This sounds reasonable to me.

Member

karmatosed commented Jul 24, 2018

How about we simply keep the unsaved changes when the block is unselected?

This sounds reasonable to me.

@gziolo

This comment has been minimized.

Show comment
Hide comment
@gziolo

gziolo Aug 1, 2018

Member

This PR needs to be rebased.

Member

gziolo commented Aug 1, 2018

This PR needs to be rebased.

@gziolo gziolo added this to the 3.8 milestone Aug 30, 2018

@gziolo

This comment has been minimized.

Show comment
Hide comment
@gziolo

gziolo Aug 30, 2018

Member

@Soean - do you plan to continue working on it or should someone else take over it?

Member

gziolo commented Aug 30, 2018

@Soean - do you plan to continue working on it or should someone else take over it?

@Soean

This comment has been minimized.

Show comment
Hide comment
@Soean

Soean Aug 30, 2018

Member

@gziolo I will work on my open pull request tomorrow

Member

Soean commented Aug 30, 2018

@gziolo I will work on my open pull request tomorrow

@gziolo

This comment has been minimized.

Show comment
Hide comment
@gziolo

gziolo Aug 30, 2018

Member

Feel free to remove the label when it gets updated 💯

Member

gziolo commented Aug 30, 2018

Feel free to remove the label when it gets updated 💯

@youknowriad youknowriad modified the milestones: 3.8, 3.9 Sep 5, 2018

@youknowriad youknowriad removed this from the 3.9 milestone Sep 13, 2018

@Soean Soean removed the [Status] Stale label Oct 9, 2018

@jasmussen

This comment has been minimized.

Show comment
Hide comment
@jasmussen

jasmussen Oct 10, 2018

Contributor

Sorry for missing this, but the discussion seems to be in good hands. I agree with all of you.

Curious if @jasmussen has any ideas here—from memory, the Edit / Cancel / Save flow originally came from a conversation that we had last year 🙂

No discussions, and the idea of simply keeping the unsaved changes when clicking out also seems to jive with my gut. The alternative would be to create an "are you sure" dialog when de-selecting, which is intrusive.

Contributor

jasmussen commented Oct 10, 2018

Sorry for missing this, but the discussion seems to be in good hands. I agree with all of you.

Curious if @jasmussen has any ideas here—from memory, the Edit / Cancel / Save flow originally came from a conversation that we had last year 🙂

No discussions, and the idea of simply keeping the unsaved changes when clicking out also seems to jive with my gut. The alternative would be to create an "are you sure" dialog when de-selecting, which is intrusive.

@gziolo

I think e2e tests for reusable blocks are failing after this change was introduced. It feels like they need to be updated after Cancel button was removed.

I will add this PR to 4.1 milestone.

@gziolo gziolo added this to the 4.1 milestone Oct 10, 2018

@Soean

This comment has been minimized.

Show comment
Hide comment
@Soean

Soean Oct 10, 2018

Member

Thanks for your feedback, I fixed the tests.

Member

Soean commented Oct 10, 2018

Thanks for your feedback, I fixed the tests.

@gziolo

gziolo approved these changes Oct 10, 2018

It looks like all feedback was addressed, let's get it in for 4.0 since it's ready.

@gziolo gziolo merged commit 2c49be5 into master Oct 10, 2018

2 checks passed

codecov/project 49.48% (-0.02%) compared to b869b99
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@gziolo gziolo deleted the update/edit-save-buttons-2 branch Oct 10, 2018

@gziolo gziolo modified the milestones: 4.1, 4.0 Oct 10, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment