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

Membership updates to correct broken layouts when multiple roles assigned #2283

Closed
wants to merge 1 commit into from

Conversation

sg00dwin
Copy link
Member

@sg00dwin sg00dwin commented Oct 17, 2017

The current layout breaks are occurring between 768-1200px when there are 3+ roles assigned. And are especially severe in edit mode .

To fix these, roles are stacked and allowed to flex-wrap. And in edit mode from 768-991px, the select field is positioned at the bottom of the stack. Also more space is given to the col-name column when the vertical nav is collapsed so that long user names don't wrap.

Before and after screens below.

Fixes #1219

Before
6

After
6b


Before
3

After
3b


Before
7

After
7b


Before
5

After
5b

@openshift-ci-robot openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 17, 2017
Copy link
Contributor

@benjaminapetersen benjaminapetersen left a comment

Choose a reason for hiding this comment

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

Tested, works well for me, big improvements at the breakpoints that used to get ugly.

Copy link
Member

@rhamilto rhamilto left a comment

Choose a reason for hiding this comment

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

I noticed some mis-alphabetizations around the new code you're adding, but would love to see a general cleanup of the issue throughout the file.

@media(min-width: @screen-md-min) {
padding: 0;
}
}
.add-role-to {
Copy link
Member

Choose a reason for hiding this comment

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

Nit: alpha

display: none;
}
}
.col-heading .col-add-role h3 {
Copy link
Member

Choose a reason for hiding this comment

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

Nit: alpha

@media(min-width: @screen-xs-min) {
max-width: 30%;
padding: 0 5px 0 0;
input {
Copy link
Member

Choose a reason for hiding this comment

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

Nit: alpha

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 18, 2017
@sg00dwin
Copy link
Member Author

@rhamilto ready for second look.
overall code cleanup and rebased

Copy link
Member

@rhamilto rhamilto left a comment

Choose a reason for hiding this comment

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

Love the well-organized Less changes. SOOO much easier to read.

@spadgett
Copy link
Member

@sg00dwin should be ready, except needs rebase

@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 20, 2017
@sg00dwin
Copy link
Member Author

@spadgett rebased and ready

@openshift-ci-robot
Copy link

@sg00dwin: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
ci/openshift-jenkins 07046e5 link /test

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@spadgett
Copy link
Member

Built /dist does not match what is committed, run 'grunt build' and include the results in your commit.

@spadgett
Copy link
Member

I cherry-picked this commit and rebuilt the dist in #2344

@spadgett spadgett closed this Oct 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

On membership, in edit mode, on the edge between tablet & larger screens, there is a potential layout wobble
6 participants