Skip to content
This repository was archived by the owner on Feb 5, 2025. It is now read-only.

Removing css that caused issue with borders displaying in secondary a…#434

Merged
andresgalante merged 1 commit intopatternfly:masterfrom
jgiardino:VerticalNav
Aug 24, 2016
Merged

Removing css that caused issue with borders displaying in secondary a…#434
andresgalante merged 1 commit intopatternfly:masterfrom
jgiardino:VerticalNav

Conversation

@jgiardino
Copy link
Copy Markdown
Contributor

Description

A previous update, #400, to list-group.less resulted in unwanted borders displaying for list group items in the secondary and tertiary nav panels. To fix the issue I removed the css that resulted in these borders displaying.

NOTE, the border style updates in #400 modified the display of borders in the list view, so that when a row was selected, a top AND bottom blue border displayed. With this CSS removed, only a top blue border will display for selected items (for any row except first). In the design document, blue borders do not display at all so I'm not sure what the actual expected look is. If further updates are needed for this, please let me know.

I also moved the css that fixed the original issue addressed by #400 from list-group.less to list-view.less as suggested by Andres.

Examples of updates

Navigation Before
screen shot 2016-08-24 at 12 36 43 pm

Navigation After
After

List View Before
screen shot 2016-08-24 at 12 37 24 pm

List View After
After

…nd tertiary nav panels. Also moving the z-index fix for PTNFLY-1134 from list-group.less to list-view.less
@jeff-phillips-18
Copy link
Copy Markdown
Member

LGTM 👍

@andresgalante
Copy link
Copy Markdown
Member

@jgiardino thanks for fixing this. 😄

@andresgalante andresgalante merged commit 28230d0 into patternfly:master Aug 24, 2016
@jgiardino jgiardino deleted the VerticalNav branch August 25, 2016 12:48
@junezhang
Copy link
Copy Markdown
Member

junezhang commented Aug 31, 2016

@jgiardino @jeff-phillips-18 @andresgalante

Thanks for reviewing the code for #400. I think if selected a row, the top and down border are both displayed will be better than only displayed the top border.

list-view-border

May I move this piece of code to list-view.css? Then the border will display well.

list-view-border-after

And it will not affect the vertical navigation.

@andresgalante
Copy link
Copy Markdown
Member

@kybaker Can you please take a look at this one?
Adding the border has a code cost that I don't think it makes a huge difference. The question is: how much do we want that border?

@jgiardino
Copy link
Copy Markdown
Contributor Author

I agree with Andres. I don’t have a strong opinion about the border, and defer to the visual design team on this. But my preference is to not add extra css unless it provides benefit to the UI.

@serenamarie125
Copy link
Copy Markdown
Member

How costly is the code cost @andresgalante ? It looks like a bug if it's omitted, which generates a user cost ;)

@jgiardino
Copy link
Copy Markdown
Contributor Author

In addition to having extra css code, the css that’s added also has a performance impact in that each row selection requires a reflow and repaint of the screen due to the styles that are being applied.

If the final decision is to keep the top/bottom border for list row selection, then it would be nice to see if we can refactor what styles are applied so that we reduce the performance impact as much as possible, while achieving the same visual design. Specifically, I'm wondering if we could apply changes to border color only, and not to border width and margin. Border color requires only a repaint (https://csstriggers.com/border-bottom-color), whereas changing border width and margin width require both reflow and repaint (https://csstriggers.com/border-bottom-width).

@LHinson
Copy link
Copy Markdown
Member

LHinson commented Aug 31, 2016

@jgiardino two questions...

  1. Will the refactor be a solution that is just as optimal as the former suggestion to just remove the boarder?
  2. How large of an effort is the refactor?

@jgiardino
Copy link
Copy Markdown
Contributor Author

I'm interested in Andres' or anyone's opinion on this, but the following is
one way to modify the existing css to achieve the top/bottom blue border
for selected rows:

  • for .list-group-item
    • change the border color to transparent
    • add background-clip: padding-box so that the background color of
      the parent .list-group would be visible through the transparent borders.
  • for .list-group
    • set the background color to #f5f5f5 (this is the current
      .list-group-item border color), this color would be visible through the
      transparent borders so that you have the same visual design as today with
      the light gray borders separating the list items
  • a few more minor styles adjustments would be needed to fix the top
    border of the list and first list item, but the rest of the styles that
    affect border color on selected rows would automatically show the top and
    bottom borders as blue.

Jenn Giardino
Senior Interaction Designer
User Experience Design Team
Red Hat, Inc.
1-919-716-5045
jgiardin@redhat.com

On Wed, Aug 31, 2016 at 2:52 PM, Leslie notifications@github.com wrote:

@jgiardino https://github.com/jgiardino two questions...

  1. Will the refactor be a solution that is just as optimal as the former
    suggestion to just remove the boarder?
  2. How large of an effort is the refactor?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
patternfly/patternfly#434 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AUFmoCDE8fpS8ClIqDuETVoIN04-xvgpks5qlc12gaJpZM4JsNdO
.

@andresgalante
Copy link
Copy Markdown
Member

@jgiardino background-clip: padding-box would do the trick 👍

@kybaker
Copy link
Copy Markdown
Contributor

kybaker commented Sep 1, 2016

@andresgalante @jgiardino I actually like the single border option. The two border option looks heavy handed with such a dark blue. The single border feels light, clean, and modern. If it saves on performance all the better.

@LHinson
Copy link
Copy Markdown
Member

LHinson commented Sep 1, 2016

@andresgalante @jgiardino @kybaker @serenamarie125 Sounds like we have a majority rule here to go with the single boarder option. I appreciate that it will improve performance, we have visual design approval and I don't believe the single line will impact the usability of selection on list view. If concerns arise, we will address that at a later date.

@junezhang
Copy link
Copy Markdown
Member

junezhang commented Sep 2, 2016

@kybaker, Sorry, one more question, currently I'm also doing a task of list view, so I'd like to confirm about the single border, should it be the bottom border or top border? or either is okay?
Currently, it is a top border when selecting an item.
https://rawgit.com/patternfly/patternfly/master-dist/dist/tests/list-view.html

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants