-
Notifications
You must be signed in to change notification settings - Fork 9
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
Overlap Changes #93
Overlap Changes #93
Conversation
Overlap found with: .navbar .dropdown-menu .alert .active .hidden Changed to: .wc-navbar .wc-dropdown-menu wc-.alert .wc-active .wc-hidden
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to update js in /src
folder where these classes are assigned.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Saw a few of the old classes in the build, I'll try and find them in src and request Spencer's review
@@ -5,7 +5,7 @@ | |||
export function layout() { | |||
var wrapper = this.wrap | |||
.append('div') | |||
.attr('class', 'aeTable row-fluid') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@samussiah I don't understand this update, so wanted to draw your attention to it - this isn't on the list in the pr comment and the class row-fluid still remains below
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't look like we need it, although it is a bootstrap class. Go ahead and remove row-fluid, wc-row-fluid, and form-inline. They're not referenced anywhere else.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@samussiah Sounds good, removed those classes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Much obliged.
@samussiah @emmorris I do not see any overlapping of columns on any of the three browsers. I compared them side by side with one having bootstrap toggled off and one with it on. There were some small css differences but nothing that was a big concern (coloring of Category names, Search bar in center of the page). When details are expanded and bootstrap is off they are sort of smushed together but not overlapped, basically, it doesn't look as nice. If you'd like me to test it in another way, I can do that, otherwise I think it's good. |
Nope, that'll do it! |
I agree that should be sufficient testing. I looked them over a bit and saw the same types of differences. With Bootstrap on:
These are all minor and I confirmed that all of them except the vertical spacing and horizontal line ones are already in the latest version of the renderer in RhoMAP, so I don't think this CSS update negatively impacted anything. We can discuss whether or not the other differences are worth addressing later. |
Overlap found with:
.navbar
.dropdown-menu
.alert
.active
.hidden
Changed to:
.wc-navbar
.wc-dropdown-menu
wc-.alert
.wc-active
.wc-hidden