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

initial draft of a kebab pattern and a card pattern #26

Merged
merged 5 commits into from
Nov 7, 2016

Conversation

jgiardino
Copy link

@jgiardino jgiardino commented Nov 2, 2016

Addresses Kebab #22

Summary of changes:

  1. updated dropdown pattern to include parameters for:

    • REMOVED: displaying dropdown as a kebab -- this will be included as a separate story
    • displaying dropdown button on the right
  2. included two variations of a card pattern

    • REMOVED: one with separate components for the card header and the card footer. This allows us to build card variations that use other components as the card contents. THIS WILL BE HANDLED WITH TRANSCLUSION (i.e. partial block)
    • one single component that includes parameters for including specific elements of the card. Card contents would be specified using the partial block, e.g. {{#> components-card ...}} card contents here {{/ components-card}}

@jgiardino
Copy link
Author

jgiardino commented Nov 2, 2016

@andresgalante @srambach -- Let me know if you have any suggestions. I realize there is a lot going on in this one, but there's a lot I'd like to get your feedback on.

I still want to include different variations of the card as currently represented on the patternfly site, but I'm not sure what direction to take the card (as a split out component where we use a card top component, other componenets for the card contents, and a card bottom component, or not). See my comments about this above.

I also wanted you to see the kebab in the context of the card, and not just by itself.

Some questions I have are:

  • Which variation of the card component should I use?
  • What layouts of cards should I include?
  • Where should styles for the kebab go? I included the same css in two different files, one for icons and one for dropdowns (i'm not sure where to add the dropdown scss file though). I will remove the updates from the one they don't belong in.
  • What styles should I use for the kebab (specifically color)? Should the blue color defined for the kebab on hover, etc... be removed (i.e. use the color defined for .btn-link on hover)?

Thanks!

.dropdown-toggle::after {
display: none;
}
.btn-link {
Copy link
Author

Choose a reason for hiding this comment

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

related to this discussion: https://github.com/patternfly/patternfly-atomic/pull/25/files#r86324052

Instead of doing this:
.pf-dropdown__kebab .btn-link

I would use something like this?:
.pf-dropdown__kebab__btn-link

Is this the correct use of dashes and underscores?

Copy link
Member

Choose a reason for hiding this comment

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

It can get messy quickly. :( Definitely if they would have been nested, I think they'd be joined by the __ as a block__element relationship. But, is the kebab a child of the pf-dropdown, or is it a variant of a pf-dropdown? So maybe it's actually .pf-dropdown--kebab__btn-link. Which looks backwards but reading it carefully you'd know it's a btn-link within the kebab variant of a .pf-dropdown? WDYT?

@andresgalante
Copy link
Member

@jgiardino A challenge we are having with the kebab on pf3 is the tiny click zone. We should fix it in pf4

@jgiardino
Copy link
Author

Oh, yes. Thank you for reminding me!

@jgiardino
Copy link
Author

jgiardino commented Nov 4, 2016

@andresgalante, @srambach, @bleathem

I made changes based on our discussion this morning about using partial blocks to include additional card elements. I didn't include an example of a card using other components as contents yet, but the partial block is defined as part of the card.

I also created separate files showing different variations of the card. Let me know if there are other variations I should include or any variation I should remove.

Some questions/issues that still need to be addressed:

  • Kebab styling - Kebab styling still needs to be updated to enlarge the clickable area (or is this something that would be covered by Sarah's implementation of the icon button?)
  • checkbox styling -
    • my biggest issue with this is how to make the clickable area larger. I made the width and height bigger which is clickable, but there's no way to position the checkbox within this area. I'm wondering if we should include the checkbox inside an empty label that's much larger than the checkbox
    • I included classes .pf-card--multiselect__block and .pf-card--multiselect__header to define padding inside these elements when checkboxes are included, so that the contents in these elements do not overlap the checkbox. But I feel like this is a case where coding guidelines clash with ease of implementation. For example, with this implementation, we're telling developers "If you want multiselection, then add this class here, and that class there" when really as a developer it's easier to just add a class to the parent element, then add the checkbox
      • this is a reminder for myself, that whatever we decide, I need to make sure the same padding that's used in the header is also included in the contents block
  • Headers - I noticed the styles for the headers changed (I think the line height was reduced). Currently the styling of headers relative to the card seems off, but I'm not sure if the header styles are final or not, and also am curious what your thoughts are.

top: $pf-spacer-xxxs;
left: $pf-spacer-xxs;
input[type=checkbox] {
//visibility: hidden; is not keyboard accessible and should be replaced with something that keeps the checkbox "visible" in the dom but not rendered on the screen
Copy link
Member

Choose a reason for hiding this comment

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

Let's document this. Can we create a mixin so we can reuse the same solution to hide/show elements in different components?

// using z-index to push the checkbox behind the card until it has keyboard focus, or the user hovers over the card
z-index:-1;
position: relative;
width: $pf-spacer-xxxl;
Copy link
Member

Choose a reason for hiding this comment

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

changing the size of an checkbox it's not consistent in different browser. Does it look bad if we use the default?

…ern and multiselection from card pattern, and included utility classes to add margin-bottom to the icons in the card
@jgiardino
Copy link
Author

@andres, @srambach
I made updates to the PR to remove card variations that use the checkbox and the kebab, until those patterns are finalized.
I also included a utility class to the icon to add margin-bottom.

@andresgalante please check the css file for components and make sure the updates are what you expected.

Thanks!

@@ -2,7 +2,7 @@
// Base styles
//

.card {
.pf-card {
Copy link
Member

Choose a reason for hiding this comment

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

all cards (including panels) have the shadow. So that one should actually be .card, sorry

| Class | Usage |
| -- | -- |
| `.pf-card--accented` **Applied to:** `.card` | **Outcome:** Displays a top border with an accent color **Required:** No |
| `.pf-card--multiselect` **Applied to:** `.card` | **Outcome:** Alters the card layout to accommodate a checkbox **Required:** If displaying a checkbox in the card |
Copy link
Member

Choose a reason for hiding this comment

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

Should there be a .pf-card-multiselect in the scss?

@andresgalante
Copy link
Member

Lets merge it, I've created a story to continue with it: https://patternfly.atlassian.net/browse/PTNFLY-1961

thanks @jgiardino you are awesome 😄

@andresgalante andresgalante merged commit 4db3727 into patternfly:master Nov 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants