Skip to content

Conversation

@christiemolloy
Copy link
Member

close #133

@patternfly-build
Copy link
Collaborator

patternfly-build commented Jan 23, 2019

Deploy preview for pf-next ready!

Built with commit fc90d48

https://deploy-preview-1288--pf-next.netlify.com

@srambach
Copy link
Member

Two things on a quick initial look -

  • The clear X is scrunched up to the edge; is this correct behavior?
  • Looks like the search button is missing (white on white) in dark mode.

image

@christiemolloy
Copy link
Member Author

@srambach thanks! Those issues relate to the input group component that I used. I can create an issue to fix those.

@@ -0,0 +1,7 @@
<ul class="pf-c-context-selector__menu-menu{{#if context-selector-menu-menu--modifier}} {{context-selector-menu-menu--modifier}}{{/if}}"
Copy link
Member

Choose a reason for hiding this comment

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

Instead of pf-c-context-selector__menu-menu, could it be called pf-c-context-selector__menu-list to be a little less like Little Caesar's Pizza Pizza? 🤣

Copy link
Member Author

Choose a reason for hiding this comment

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

yes haha !

@mcarrano
Copy link
Member

When I open this in Firefox, the input group is overlapping its container.

screen shot 2019-01-23 at 2 34 43 pm

Interestingly, in Chrome the spacing I see is different from @srambach and actually looks OK.

screen shot 2019-01-23 at 2 39 45 pm

align-items: center;
justify-content: space-between;
min-width: var(--pf-c-context-selector__toggle--MinWidth);
min-width: 100%;
Copy link
Contributor

Choose a reason for hiding this comment

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

why are there 2 min-widths?

Copy link
Member Author

Choose a reason for hiding this comment

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

no idea, keeping the second one

margin-left: var(--pf-c-context-selector__toggle-icon--MarginLeft);
}

.pf-c-context-selector__toggle-text {
Copy link
Contributor

Choose a reason for hiding this comment

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

if you put a ton of text in here, it just makes the button wider and doesn't truncate.

Copy link
Member Author

Choose a reason for hiding this comment

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

awesome added a max-width

@@ -0,0 +1,145 @@
{{#> context-selector id="context-selector-collapsed-example" context-selector--label-text="My Project"}}
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think the label text and toggle text should match... I'm not sure how to handle this though. The label might read something like "Selected project"?

Copy link
Member Author

Choose a reason for hiding this comment

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

going to go with that

| `.pf-c-context-selector__menu-input` | `<div>` | Initiates a container for the input group. |
| `.pf-c-context-selector__menu-menu` | `<ul>` | Initiaties an unordered list of menu items that sits under the input container. |
| `.pf-c-context-selector__menu-menu-item` | `<li>` | Initiaties a menu item. |
| `.pf-m-expanded.` | `.pf-c-context-selector` | Modifies for the expanded state. |
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
| `.pf-m-expanded.` | `.pf-c-context-selector` | Modifies for the expanded state. |
| `.pf-m-expanded` | `.pf-c-context-selector` | Modifies for the expanded state. |

@christiemolloy
Copy link
Member Author

@mcoker made updates here, I can't seem to fix the issue that @mcarrano bought up about the input-group overflowing only in firefox :(

--pf-c-context-selector__toggle--PaddingRight: var(--pf-global--spacer--sm);
--pf-c-context-selector__toggle--PaddingBottom: var(--pf-global--spacer--form-element);
--pf-c-context-selector__toggle--PaddingLeft: var(--pf-global--spacer--sm);
--pf-c-context-selector__toggle--MinWidth: var(--pf-global--target-size--MinWidth);
Copy link
Contributor

Choose a reason for hiding this comment

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

doesn't look like this is used

Copy link
Member Author

Choose a reason for hiding this comment

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

updated!

Copy link
Contributor

@mcoker mcoker left a comment

Choose a reason for hiding this comment

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

🥳

--pf-c-context-selector__menu-list-item--disabled--Color: var(--pf-global--Color--dark-200);
--pf-c-context-selector__menu-list-item--disabled--BackgroundColor: transparent;


Copy link
Contributor

Choose a reason for hiding this comment

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

can you add @extend %pf-t-light; and remove the transparent backgrounds on lines 12 and 61. This will fix the theme issue.

@matthewcarleton matthewcarleton merged commit 7183a80 into patternfly:master Jan 25, 2019
@patternfly-build
Copy link
Collaborator

🎉 This PR is included in version 1.0.161 🎉

The release is available on:

Your semantic-release bot 📦🚀

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.

6 participants