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

Apply new checkbox style to admin settings #19744

Merged
merged 1 commit into from Oct 14, 2015
Merged

Conversation

Henni
Copy link
Contributor

@Henni Henni commented Oct 13, 2015

@MorrisJobke
Copy link
Contributor

cc @owncloud/designers @PVince81

@MorrisJobke
Copy link
Contributor

Tested and works 👍

@MorrisJobke MorrisJobke added this to the 9.0-next milestone Oct 13, 2015
@PVince81
Copy link
Contributor

Code looks good 👍

@jancborchardt
Copy link
Member

Isn’t this also something which we should backport to 8.2 then? cc @karlitschek @cmonteroluque because atm in some places we have the new checkboxes, and in some we don’t, which is not very elegant.

@karlitschek
Copy link
Contributor

yes. backport please. honestly i'm surprised that this wasn't fixed a long time ago

@jancborchardt
Copy link
Member

Yeah, I was also confused pulling the new code coming back from vacation. ;) But great work @Henni! :)

@PVince81
Copy link
Contributor

the explanation is simple: the initial fix touched ALL the checkboxes that have a "label" tag. But since some apps didn't have them, their checkboxes were broken. So the initial fix was not acceptable due to app breakage and was reverted, and now checkbox styles need to be opt-in. So all checkboxes that used to work with the old approach would need to be adjusted directly, and some might have been missed during the second pass.
See #19304

@jancborchardt
Copy link
Member

@Henni can you rebase so we can merge and backport? :)

@Henni
Copy link
Contributor Author

Henni commented Oct 14, 2015

@jancborchardt done

DeepDiver1975 added a commit that referenced this pull request Oct 14, 2015
Apply new checkbox style to admin settings
@DeepDiver1975 DeepDiver1975 merged commit 24011d8 into master Oct 14, 2015
@DeepDiver1975 DeepDiver1975 deleted the admin-checkboxes branch October 14, 2015 10:37
@nickvergessen
Copy link
Contributor

@Henni please send a backport Pull request against stable8.2

@Henni
Copy link
Contributor Author

Henni commented Oct 14, 2015

@nickvergessen I'm not completely sure how to do this, but I'll try.

@nickvergessen
Copy link
Contributor

git checkout stable8.2
git checkout -b backport-19744
git cherry-pick bf722d9
git push origin backport-19744

And then send a PR against stable8.2

karlitschek added a commit that referenced this pull request Oct 15, 2015
Backport admin checkboxes #19744 to stable8.2
@lock lock bot locked as resolved and limited conversation to collaborators Aug 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New-style checkboxes lost again in some places
7 participants