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

CRM-21055 Revert change of "cancel" to "close" on backend contribution form #11064

Merged
merged 1 commit into from
Oct 5, 2017

Conversation

agh1
Copy link
Contributor

@agh1 agh1 commented Oct 4, 2017

This reverts commit 35c930c.

The change was done in isolation, leading to inconsistency with other forms
that have the Save / Save and New / Cancel pattern.

@totten @colemanw please take a look before shipping 4.7.25--I just noticed #10845 as I was writing up the release notes and I'm very concerned about it. See the explanation on JIRA

Basically, either all instances of this pattern should say "Close" or all should say "Cancel". In the meantime, we shouldn't be shipping it with the one exception, especially since it changes longstanding behavior.

@eileenmcnaughton @pradpnayak @monishdeb @lcdservices

This reverts commit 35c930c.

The change was done in isolation, leading to inconsistency with other forms
that have the Save / Save and New / Cancel pattern.
@JoeMurray
Copy link
Contributor

I don't think this should be hastily reverted. See issue for discussion.

@eileenmcnaughton
Copy link
Contributor

eileenmcnaughton commented Oct 4, 2017

Per comments on Chat I AM going for a hasty revert. I don't think this was done hastily as it has been in the rc for a month and in master for a while before that, or without discussion. However, I think it only provides a trivial benefit and trivially affects lots of people so caution is the better part of valour here

@eileenmcnaughton
Copy link
Contributor

hmm looks like we missed the release!

@totten
Copy link
Member

totten commented Oct 5, 2017

There seems to be general support for reverting the label change. Joe's point is good that it wouldn't justify reissuing a release on its own -- but we're doing a reissue anyway due to the release-notes.

@totten totten merged commit 54096a1 into civicrm:4.7.25-rc Oct 5, 2017
@eileenmcnaughton
Copy link
Contributor

Note that this might yet get re-introduced - but I think we need to discuss further first since Andrew had some good points (to be reconciled with good points made by others in favour of this change)

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

Successfully merging this pull request may close these issues.

5 participants