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

Grid layout config: Replace the old checkbox with the umb-checbox directive #6859

Merged
merged 2 commits into from
Oct 29, 2019
Merged

Grid layout config: Replace the old checkbox with the umb-checbox directive #6859

merged 2 commits into from
Oct 29, 2019

Conversation

BatJan
Copy link
Contributor

@BatJan BatJan commented Oct 25, 2019

Prerequisites

  • I have added steps to test this contribution in the description below

Description

I have replaced the old checkbox with the umb-checkbox directive - I have also temporarily added some inline styling to align the checkbox. But that's only until this PR is merged #6858

So before merging this PR please merge the other one and let me update this PR with a proper modifier CSS class 😉

You might also wonder "Why not change the other checkboxes that are revealed when the new one is toggled?" - Well, that requires an extension to the umb-checkbox directive, which I'm having a think about these days to make it as flexible as we need it to be. Stay tuned 👀

Before
grid-layout-before

After
grid-layout-after

@bjarnef
Copy link
Contributor

bjarnef commented Oct 25, 2019

Maybe umb-toggle would be better in this case since it is more a setting.

Copy link
Member

@nul800sebastiaan nul800sebastiaan left a comment

Choose a reason for hiding this comment

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

I would agree with Bjarne here, this one is better suited to be a toggle. Could you update it to be that please @BatJan?

@BatJan
Copy link
Contributor Author

BatJan commented Oct 29, 2019

@nul800sebastiaan Done 😉

@nul800sebastiaan nul800sebastiaan merged commit e906704 into umbraco:v8/dev Oct 29, 2019
@nul800sebastiaan
Copy link
Member

Lovely, thanks! 👍

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.

6 participants