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

lib/form-layout: Add support for widgets nested in splits #11154

Closed

Conversation

garrett
Copy link
Member

@garrett garrett commented Feb 11, 2019

Fix alignment as mentioned in #11083 by applying special widget offseting & min-height to widgets also directly enclosed within ct-form-layout-split, which should behave now similarly to top-level children.

@garrett
Copy link
Member Author

garrett commented Feb 11, 2019

Ideally, the widget would be a direct child of the grid element. That's what splits are designed for. However, in looking at the code, it would be difficult to "fix" the HTML to work as intended, so I basically adjusted the CSS to account for cases like this, as it'll probably show up again in the future too (and isn't really "wrong").

@garrett
Copy link
Member Author

garrett commented Feb 11, 2019

Oh, this will probably clash with b384c4b, which hasn't been merged yet. That specific commit is similar and we can add this same logic to b384c4b#diff-065d8dbbc10bd4f648805392b2b935f6R137 (line 137)

@garrett garrett force-pushed the form-layout-split-widgets branch from c5d01ac to 274d21c Compare February 11, 2019 14:28
@garrett
Copy link
Member Author

garrett commented Feb 11, 2019

Merged the two commits into one.

@andreasn
Copy link
Contributor

The text is still one pixel off, but maybe that's a general problem with checkboxes? Look a lot better with this PR applied, in general, though.

screenshot from 2019-02-11 17-59-52

@garrett
Copy link
Member Author

garrett commented Feb 11, 2019

The text is still one pixel off

Oh, hmm. I should investigate this. 😉

KKoukiou
KKoukiou previously approved these changes Feb 12, 2019
@garrett
Copy link
Member Author

garrett commented Feb 12, 2019

Fixed checkboxes and radios in various scenarios and added list-group-item fixes here now too. (Network page among others had odd stuff going on. I had some local fixes in another PR, but it really belongs here.)

@garrett
Copy link
Member Author

garrett commented Feb 12, 2019

I need to do further testing tomorrow and perhaps some little fixups, but wanted to come to a stopping point today with something that should (mostly?) work as expected.

andreasn
andreasn previously approved these changes Feb 12, 2019
Copy link
Contributor

@andreasn andreasn left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@garrett
Copy link
Member Author

garrett commented Feb 13, 2019

The changes here and in #11035 don't work well together. I'll have to investigate further to make everything work. It might be local to that PR, or perhaps I'm making wrong assumptions somewhere. 😒

@martinpitt
Copy link
Member

Sorry, this fell through the cracks. This needs to be rebased.

@garrett
Copy link
Member Author

garrett commented Mar 7, 2019

That's OK: I'm not quite done with it either, if I remember correctly. I got pulled over to other PRs, interacting with other people on the other issues was more important than this fixup PR.

I'm also pretty certain I did some commit collisions with myself. 😉

@garrett
Copy link
Member Author

garrett commented Mar 14, 2019

Rebased, as to not let this get too stale; need to check (and fix) it everywhere still.

@garrett garrett force-pushed the form-layout-split-widgets branch from 93e1e7d to e03a283 Compare March 14, 2019 16:50
@garrett
Copy link
Member Author

garrett commented Mar 14, 2019

This merged version still has a few errors:

Screenshot_2019-03-14 Storage - Sunny

Screenshot_2019-03-14 Storage - Sunny(1)

And things are probably off a pixel or two elsewhere (checkboxes, most likely). Still needs work.

@garrett
Copy link
Member Author

garrett commented Apr 2, 2019

I really need to find the time to finish this and de-conflict-ify it (again), then fix it up again. We still have a few bugs in Cockpit that are addressed in this PR.

@garrett garrett force-pushed the form-layout-split-widgets branch from e03a283 to 4b04b73 Compare April 8, 2019 09:39
@marusak
Copy link
Member

marusak commented Jul 23, 2019

Needs to rebase to master since #12367 changed tests names

@garrett
Copy link
Member Author

garrett commented Jul 23, 2019

I think I basically implemented this either during the PF4-style refresh or shortly before.

@garrett garrett closed this Jul 23, 2019
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