feat(search): add support for expandable variant #8101

janhassel · 2021-03-16T15:38:32Z

Closes #6360

Adds an expandable variant to the search component.

Changelog

New

Expandable variant / kind for search component

Changed

Renamed previous bx--search-magnifier classname to bx--search-magnifier-icon
- If changing classnames is considered a breaking change, I can also revert this and change the new bx--search-magnifier to bx--search-magnifier-container – just let me know!
Updated API snapshot to reflect new prop

Testing / Reviewing

When props.kind = 'expandable', the search component should

be collapsed by default
open by either
- clicking the search icon
- tabbing into it / receiving focus
close when it loses focus and has no content
stay open if it loses focus and has content

netlify · 2021-03-16T15:49:14Z

Deploy preview for carbon-elements ready!

Built with commit f442072

https://deploy-preview-8101--carbon-elements.netlify.app

netlify · 2021-03-16T15:59:47Z

Deploy preview for carbon-components-react ready!

Built without sensitive environment variables with commit f442072

https://deploy-preview-8101--carbon-components-react.netlify.app

tay1orjones

The alignment is off in DataTable in the button/hover icon alignment, the alignment of the button when it's expanded, and the interior left padding of the input when expanded.

tay1orjones · 2021-03-19T20:14:05Z

packages/components/src/components/search/_search.scss

+    width: 0;
+    padding: 0;
+    transition-timing-function: motion(standard, productive);
+    transition-duration: $duration--fast-01;
+    transition-property: width, padding;


Cool, so it looks like this will inherently support animating leftwards, rightwards, or both from the center 👍

joshblack · 2021-03-22T16:54:28Z

packages/react/src/components/Search/Search.js

+    /**
+     * Optional prop to specify the kind of this component
+     */
+    kind: PropTypes.oneOf(['default', 'expandable']),


One quick question, curious what you think between using kind props versus named variants of a component.

e.g. we have <Search kind="expandable" /> or <ExpandableSearch />

I've definitely leaned towards the latter since the kind prop makes it harder to group related functionality around a variant but I'm super curious what you think @janhassel

Could the prop name also be something like isExpandable?

Or we could mimic the TableToolbarSearch prop, and use persistent and default that to true? (perhaps renamed in both places to isPersistent in v11)

I initially thought this variant would not be big enough to make into its own component but after looking further into some other components, maybe the guidance could be to use kind whenever the appearance is the main differentiator and creating a new component whenever the behaviour is affected. In that case, this should be an <ExpandableSearch /> as @joshblack mentioned.

This is based on the following:

Component kind wrapper Effect

Button X Changes appearance: primary, secondary, ghost, danger

Notification X Changes apperance: success, error, warning, information

Tile X Changes behaviour: ClickableTile, SelectableTile, RadioTile

ContextMenu X Changes behaviour: ContextMenuItem, ContextMenuSelectableItem, ContextMenu

The (somewhat) recent change in Toggle / ToggleSmall also reflects this rule: #7380

Let me know what you think about this approach and I'll update the PR accordingly.

@janhassel that's a great breakdown. When the prop controls behavior (or even something like markup potentially) the wrapper approach works great. If it's a visual change only (like classes) then inlining in the component makes more sense.

Following that chart, does it make sense for ExpandableSearch to be a wrapper, or am I misunderstanding? 👀

@joshblack Yes, I'd say this should be a wrapper component called ExpandableSearch following above's approach. If you agree, @tw15egan then I'll go ahead and update the PR.

Let's do it! 👍🏻

@joshblack @tw15egan Just pushed an update. I tried syncing the prop types with ExpandableSearch.propTypes = Search.propTypes which seems to work fine for validation but the storybook docs don't pick them up. Do you guys have any idea on how to solve this?

janhassel · 2021-03-23T08:06:28Z

@tay1orjones Good catch! This was caused by the change class name change. Just pushed an update!

dakahn · 2021-03-30T02:15:58Z

@carbon-design-system/design just wanted to tap yall in for a sign off here
This PR adds a collapsible global search variant like the search seen in DataTable (where it expands from just a magnifying icon).

Specifically want to call attention to the animation behavior where the search expands on focus -- which seems different than what we do on carbondesignsystem.com (expand on focus is better 👍🏽)

laurenmrice

In Safari: The word Search is persistent by default next to the search icon. It should only be the icon.

janhassel · 2021-04-07T07:56:29Z

@laurenmrice Should be fixed now!

laurenmrice

looks good 👍

packages/react/src/components/ExpandableSearch/ExpandableSearch.js

joshblack

Looking great!

joshblack · 2021-04-08T17:27:41Z

bump @tay1orjones @tw15egan when you have a sec to re-review 👀

tay1orjones

Sorry, thought I had already put an approve on here.

The storybook prop table is a bummer but I don't think there's a workaround since it's probably blowing up react-docgen. The easiest way I know of to fix is to just copy/paste the proptypes 😞

feat(search): add support for expandable variant

dd4a36e

janhassel requested review from a team as code owners March 16, 2021 15:38

janhassel requested review from joshblack and dakahn March 16, 2021 15:38

test: update data-table snapshots

08e322e

tay1orjones requested changes Mar 19, 2021

View reviewed changes

joshblack reviewed Mar 22, 2021

View reviewed changes

fix(data-table): update css selectors for search magnifier icon

4fc46a3

janhassel added 2 commits March 25, 2021 11:43

refactor(search): extract expandable kind into own component

28716d1

Merge branch 'main' into expandable-search

93ac990

dakahn removed their request for review April 5, 2021 22:50

Merge branch 'main' into expandable-search

2f9e366

tw15egan requested a review from laurenmrice April 6, 2021 16:59

laurenmrice requested changes Apr 6, 2021

View reviewed changes

fix(expandable-search): fade placeholder

1bac603

janhassel requested a review from laurenmrice April 7, 2021 07:56

laurenmrice approved these changes Apr 7, 2021

View reviewed changes

joshblack reviewed Apr 8, 2021

View reviewed changes

packages/react/src/components/ExpandableSearch/ExpandableSearch.js Outdated Show resolved Hide resolved

joshblack approved these changes Apr 8, 2021

View reviewed changes

tay1orjones approved these changes Apr 8, 2021

View reviewed changes

janhassel added 2 commits April 9, 2021 09:23

fix(expandable-search): move focusInput function inside effect)

46719df

fix(expandable-search): use icon-01 as fill color when collapsed

8b7adcc

tw15egan approved these changes Apr 9, 2021

View reviewed changes

tw15egan added the status: ready to merge 🎉 label Apr 10, 2021

Merge branch 'main' into expandable-search

f442072

kodiakhq bot merged commit df0d1e0 into carbon-design-system:main Apr 10, 2021

janhassel deleted the expandable-search branch April 10, 2021 05:46

tw15egan mentioned this pull request Apr 20, 2021

Search Component Magnifier Icon Has Mixed Class Selectors #8460

Closed

kaushalnavneet mentioned this pull request Apr 29, 2021

TableToolbarSearch needs to be updated with latest magnifier classname #8535

Closed

1 task

PPPiQ mentioned this pull request May 14, 2021

Update of bx--search-magnifier-icon in search component carbon-design-system/carbon-components-angular#1820

Merged

tw15egan mentioned this pull request Jun 1, 2021

fix(TableToolbarSearch): fix rendering issues with TableToolbarSearch #8824

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(search): add support for expandable variant #8101

feat(search): add support for expandable variant #8101

janhassel commented Mar 16, 2021

netlify bot commented Mar 16, 2021 •

edited

Loading

netlify bot commented Mar 16, 2021 •

edited

Loading

tay1orjones left a comment

tay1orjones Mar 19, 2021

joshblack Mar 22, 2021

tw15egan Mar 22, 2021 •

edited

Loading

janhassel Mar 23, 2021

joshblack Mar 23, 2021

janhassel Mar 23, 2021

tw15egan Mar 23, 2021

janhassel Mar 25, 2021

janhassel commented Mar 23, 2021

dakahn commented Mar 30, 2021 •

edited

Loading

laurenmrice left a comment

janhassel commented Apr 7, 2021

laurenmrice left a comment

joshblack left a comment

joshblack commented Apr 8, 2021

tay1orjones left a comment

Component	`kind`	wrapper	Effect
Button	X		Changes appearance: primary, secondary, ghost, danger
Notification	X		Changes apperance: success, error, warning, information
Tile		X	Changes behaviour: ClickableTile, SelectableTile, RadioTile
ContextMenu		X	Changes behaviour: ContextMenuItem, ContextMenuSelectableItem, ContextMenu

feat(search): add support for expandable variant #8101

feat(search): add support for expandable variant #8101

Conversation

janhassel commented Mar 16, 2021

Changelog

Testing / Reviewing

netlify bot commented Mar 16, 2021 • edited Loading

netlify bot commented Mar 16, 2021 • edited Loading

tay1orjones left a comment

Choose a reason for hiding this comment

tay1orjones Mar 19, 2021

Choose a reason for hiding this comment

joshblack Mar 22, 2021

Choose a reason for hiding this comment

tw15egan Mar 22, 2021 • edited Loading

Choose a reason for hiding this comment

janhassel Mar 23, 2021

Choose a reason for hiding this comment

joshblack Mar 23, 2021

Choose a reason for hiding this comment

janhassel Mar 23, 2021

Choose a reason for hiding this comment

tw15egan Mar 23, 2021

Choose a reason for hiding this comment

janhassel Mar 25, 2021

Choose a reason for hiding this comment

janhassel commented Mar 23, 2021

dakahn commented Mar 30, 2021 • edited Loading

laurenmrice left a comment

Choose a reason for hiding this comment

janhassel commented Apr 7, 2021

laurenmrice left a comment

Choose a reason for hiding this comment

joshblack left a comment

Choose a reason for hiding this comment

joshblack commented Apr 8, 2021

tay1orjones left a comment

Choose a reason for hiding this comment

netlify bot commented Mar 16, 2021 •

edited

Loading

netlify bot commented Mar 16, 2021 •

edited

Loading

tw15egan Mar 22, 2021 •

edited

Loading

dakahn commented Mar 30, 2021 •

edited

Loading