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

Adjust grid editor layout configuration #9887

Merged

Conversation

bjarnef
Copy link
Contributor

@bjarnef bjarnef commented Feb 23, 2021

Prerequisites

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

If there's an existing issue for this PR then this fixes #9825

Description

This PR fixes a few issues with configuration of layout/rows.

  • Fixed wrong end-closing tag of <button> element which was </a> + added missing type attribute on button element.
  • Registred functions on vm to be a bit more consistent with the rest of code base. I found a function closeArea in row configuration, which doesn't seem to be used, but I have leaved it for now.
  • Previous the rows in layout configuration was using traditional checkboxes along with checklist-model and checklist-value which auto-populated the allowed array with row names. However since we are using <umb-checkbox> we need to bind the value to a model, but the row itself was not unique per section/column. I have extended the sections in the layout with a rows, which basically have a copy of the rows and a selected state. This is used to bind the value on the checkbox and/or if the button element next to it is clicked. On submit this property is deleted again, so we have to original JSON.
    The might be a better way to handle this in future, but for now it seems to do the trick :)

Layout configuration

jzjtVAmXkf

Row configuration

YvozltVuL3

@nathanwoulfe
Copy link
Contributor

Relates to #9410

@madsrasmussen madsrasmussen changed the base branch from v8/contrib to v8/dev May 6, 2021 14:52
@madsrasmussen
Copy link
Contributor

Hi Bjarne,

Thanks for the PR 🎉

When testing it a found a couple of smaller things that I have fixed.

  • When adding a new section in the layouts dialog we didn't copy the rows so there was nothing to select.
  • When cleaning up rows in the submit function it looked in the wrong place.
  • I have also made sure to clean up on close

You can see my commit here:
ca4d43d

Does this look ok to you and can I get you to do an extra test with the new fixes?

@bjarnef
Copy link
Contributor Author

bjarnef commented May 6, 2021

Hi Mads

Great, I have tested the PR with your changes, and it seems to work as expected. I minor thing is when checking all rows or editors and submit the overlay and re-open it again, it could probably have the "Allow all row configurations" or "Allow all editors" toggle enabled, but I think it has always worked this way.

The input fields could be closer the labels by removing the form-horizontal class, but it may cause issues with other overlays and something that can be looked at in another PR.

image

image

@bjarnef bjarnef mentioned this pull request May 6, 2021
@madsrasmussen
Copy link
Contributor

Thanks Bjarne for taking you time to do an extra test 🥇

Yes, I agree that the overall design of the dialogs could use some more love. I can see that there is a PR from Nathan with some style clean-up. I will have to see if that also includes some UI love :)

I think this PR does what it should and I will get it merged!

@madsrasmussen madsrasmussen merged commit aaa1330 into umbraco:v8/dev May 7, 2021
@bjarnef bjarnef deleted the v8/bug/grid-editor-configuration branch May 7, 2021 08:31
@nathanwoulfe
Copy link
Contributor

Most of my changes are polish and cleanup, not substantially changing anything. Still be good to merge, nothing wrong with a bit of polish ☺️

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.

Umbraco Grid layout can only configure allowed rows for one column
5 participants