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

Input groups #961

Merged
merged 28 commits into from
Jul 13, 2018
Merged

Input groups #961

merged 28 commits into from
Jul 13, 2018

Conversation

cchaos
Copy link
Contributor

@cchaos cchaos commented Jul 2, 2018

This PR is mainly to put into place some pieces necessary to create the global nav items.

EuiFormControlLayout

You can now add one or an array of nodes to prepend or append and it will auto style like so:
screen shot 2018-07-02 at 15 58 20 pm

Only a few form controls allow these inherently and therefore, it's more of a hidden feature as it shouldn't be over-used.

EuiFilterGroup

Fixed up the styling for this and EuiFilterGroupButtons.
screen shot 2018-07-02 at 16 14 31 pm

Including the usage by EuiSearchBar.
screen shot 2018-07-02 at 16 14 57 pm

Added EuiDatePickerRange

There is now a new component to help style the look of a range of dates that is more aligned with our design files.

screen shot 2018-07-02 at 16 16 47 pm

cchaos added 21 commits June 26, 2018 16:28
- Made botttom colored border from linear-gradient
- Allowing parameters to determine if just the borders should render
- Allowing parameters to determine if all state styles should be added
- Made bottom colored border from linear-gradient
- Allowing parameters to determine if just the borders should render
- Allowing parameters to determine if all state styles should be added
- Fixing variable naming schemes
commit 2eea143
Author: cchaos <caroline.horn@elastic.co>
Date:   Thu Jun 28 12:56:56 2018 -0400

    Get rid of terrible IE highlight when select is focused

commit 66647f0
Author: cchaos <caroline.horn@elastic.co>
Date:   Thu Jun 28 12:07:54 2018 -0400

    Revert commit of filter_button.scss

commit c2e8a73
Author: cchaos <caroline.horn@elastic.co>
Date:   Thu Jun 28 12:00:54 2018 -0400

    Simplifying form control styles

    - Made bottom colored border from linear-gradient
    - Allowing parameters to determine if just the borders should render
    - Allowing parameters to determine if all state styles should be added
    - Fixing variable naming schemes

commit c5f0ab5
Author: Stacey Gammon <gammon@elastic.co>
Date:   Thu Jun 28 10:02:16 2018 -0400

    add inspect to typedef file (elastic#952)

    * add inpsect to typedef file

    * use the right pr number

commit c64f055
Author: Nathan Reese <reese.nathan@gmail.com>
Date:   Wed Jun 27 08:52:41 2018 -0600

    add MutationObserver to accordion to trigger setChildContentHeight when children change (elastic#947)

    * add MutationObserver to accordion to trigger setChildContentHeight when children change

    * remove requestAnimationFrame around MutationObserver registration and add comment to change log

    * set up observer in ref creation function

    * mock MutationObserver
Had to move this logic back into the layout comp because the icons needed to be wrapped with children
…r-input-groups

# Conflicts:
#	src/components/combo_box/_combo_box.scss
#	src/components/form/_mixins.scss
# Conflicts:
#	src/components/combo_box/_combo_box.scss
#	src/components/form/_mixins.scss
- And fixing flashing bg of empty buttons
to control the style of two date pickers.
@cchaos cchaos requested a review from chandlerprall July 2, 2018 20:18
const secureRel = getSecureRelForTarget(target, rel);
// let secureRel;
// if (href) {
// secureRel = getSecureRelForTarget(target, rel);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Question: So I just commented this out for now, but it seems to me that filter buttons should never link to webpages and so this feels unnecessary to keep.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cool, removing those props

<span className="euiFilterButton__textShift" data-text={children}>
{children}
{numFilters &&
<EuiHeaderNotification className="euiFilterButton__notification">{numFilters}</EuiHeaderNotification>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm going to make a new issue to generalize this component so it's not header specific.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This has been done and is updated

@cchaos
Copy link
Contributor Author

cchaos commented Jul 10, 2018

@chandlerprall Can you also take a look at this PR this week?

Copy link
Contributor

@chandlerprall chandlerprall left a comment

Choose a reason for hiding this comment

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

couple comments, overall this is great

@@ -19,8 +19,8 @@ const colors = [
export default () => (
<EuiFlexGroup gutterSize="s" alignItems="center">
{
colors.map(color => (
<EuiFlexItem grow={false}>
colors.map((color, index) => (
Copy link
Contributor

Choose a reason for hiding this comment

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

use the color value as the key, using index when there is a unique value available is an anti-pattern (it has potential performance implications)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

</EuiFormRow>
</EuiFlexItem>
</EuiFlexGroup>
<EuiDatePickerRange
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe add simple validation to ensure the start date is before end date?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just for the doc example?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, since validation lives outside the controls. Would be good to have a reference example.

const secureRel = getSecureRelForTarget(target, rel);
// let secureRel;
// if (href) {
// secureRel = getSecureRelForTarget(target, rel);
Copy link
Contributor

Choose a reason for hiding this comment

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

I agree

cchaos added 2 commits July 13, 2018 14:08
# Conflicts:
#	CHANGELOG.md
#	src/components/header/_header_notification.scss
Copy link
Contributor

@chandlerprall chandlerprall left a comment

Choose a reason for hiding this comment

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

LGTM!

@cchaos cchaos merged commit d54c4cc into elastic:master Jul 13, 2018
@cchaos cchaos deleted the filter-input-groups branch July 13, 2018 18:27
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.

2 participants