-
Notifications
You must be signed in to change notification settings - Fork 10
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
[Action Bar] - MVP experimental new component #733
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job 👍 I left some thoughts in the comments below.
packages/react-components/src/components/ActionBar/ActionBar.tsx
Outdated
Show resolved
Hide resolved
packages/react-components/src/components/ActionBar/ActionBar.tsx
Outdated
Show resolved
Hide resolved
packages/react-components/src/components/ActionBar/ActionBar.tsx
Outdated
Show resolved
Hide resolved
packages/react-components/src/components/ActionBar/ActionBar.tsx
Outdated
Show resolved
Hide resolved
packages/react-components/src/components/ActionBar/ActionBar.tsx
Outdated
Show resolved
Hide resolved
packages/react-components/src/components/ActionBar/ActionBar.tsx
Outdated
Show resolved
Hide resolved
packages/react-components/src/components/ActionBar/ActionBar.tsx
Outdated
Show resolved
Hide resolved
observerOptions | ||
); | ||
|
||
target.forEach((e) => observer.observe(e)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
target.forEach((e) => observer.observe(e)); | |
target.forEach(observer.observe); |
entry.target.setAttribute('tabindex', '-1'); | ||
|
||
if (!menuItemsKeys.includes(entry.target.id)) { | ||
setMenuItemsKeys([...menuItemsKeys, entry.target.id]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
setMenuItemsKeys([...menuItemsKeys, entry.target.id]); | |
setMenuItemsKeys((prevItemKeys) => [...prevItemKeys, entry.target.id]); |
}; | ||
|
||
React.useEffect(() => { | ||
if (!isScrollType) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (!isScrollType) { | |
const hasIOSupport = !!window.IntersectionObserver; | |
if (!isScrollType && hasIOSupport) { |
if (!entry.isIntersecting) { | ||
entry.target.setAttribute('tabindex', '-1'); | ||
|
||
if (!menuItemsKeys.includes(entry.target.id)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To avoid double-checking the same condition (here and next if
, in the line 68) we can store this condition in a separate constant or use if-else
statement.
|
||
return () => observer.disconnect(); | ||
} | ||
}, [menuItemsKeys]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about isScrollType
prop? Maybe we should also include it here?
/** | ||
* Set the key for active element | ||
*/ | ||
activeOptionKey?: string | null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need this Key
suffix?
activeOptionKey?: string | null; | |
activeOption?: string | null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Name references to the part of the option
object, so the key
suffix is correct for me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* #680 - Radius tokens (#690) radius tokens * #681 - Button - Updated button border radius (#691) * radius tokens * updated button border radius * new radius tokens naming * #682 - Update border-radius for components (#692) * radius tokens * update border-radius for compoenents * #687 - Modal - Refreshed UI (#700) * refreshed ui * test fix * test fix * #685 - Picker - UI refresh (#695) * picker ui refresh * fix * fix * correct border radius for tags inside picker selected options * fixes after design review * fixes after review * changes after review * #688 - Segmented Control - UI Refresh (#703) refresh ui changes * #689 - Tag input - UI refresh for tag input (#702) ui refresh for tag input * #706 - Tab - UI changes for border (#708) UI changes for border * test deploy * v1.0.0-refresh-ui.0 * test * fixes after merging * fixes * temp version for tests * ui fixes for refresh * temp version * fix for modal header * #709 - PromoBannerV2 - New component (#716) * new promo banner version * export component type, export component in main index file * changes after review * container queries + docs instructions how to use it * fix for test, defined font color for banner * informaction in the old banner about deprecation * #723 - Button - New High Contrast Button variant, styles updates (#732) * new styles for buttons, new button variant * changes in button styles * change native buttons in components to Button component from lib * fixes after review * change the button kinds * #718 - Modal - Change header token (#734) change the header token * #726 - Alert - Change buttons kind (#735) change the button kinds * #728 - Tooltip - Change default button kinds for Interactive and User Guide (#736) * change the button kinds * change default button kinds for tooltip interactive * changes in user guide * changes after review * changes after review * #719 - ActionMenu - Refresh UI + active state (#737) * change the button kinds * active state for the ActionMenu + UI refresh * fix after review * changes after review * change for dividers * #727 - PromoBannerV2 - Updates in documentation and new typography tokens (#750) * change the button kinds * changes in spacings * new typography token + docs + usage in PromoV2 * fix for spacing in small promo * #748 - Tag - New size + documentation update (#751) * new size * radius token * #749 - Button - New variant (#752) new variant * #671 Add new options to Picker (#741) * add options to Picker * add adjacent items logic and styles * [Action Bar] - MVP experimental new component (#733) * wrapper container and functionality to hide elements and move to menu * disable tabindex for hidden elements * story changes * change the button kinds * active status for buttons * changes after review * changes after review * changes * add export * changes after review * changes after review * Fix for ellipsis in Tag component (#764) changes in tag to enable ellipsis + tags in picker body * deprecate info * fix --------- Co-authored-by: jpyzio123 <46003125+jpyzio123@users.noreply.github.com>
Description
MVP of the new component called
ActionBar
. (experimental)The purpose of this component is to display the options in the form of buttons. Contrary to
SegmentedControl
this component is supposed to adapt to the container in which it is embedded and in a situation where there is not enough space to display all buttons, those that lack space will be hidden and available in the form of options in theActionMenu
.The component contains an observer that will hide or show additional buttons in case of changing the width of the container, depending on the available space. There is also an option
type: 'scroll'
that disables the menu and enables the scrolling in the main container.Remember that this is an experimental component and may not meet all user requirements at the moment. It will be extended in the future so that it can be better adapted to the design.
Storybook
https://feature-action-bar-mvp--613a8e945a5665003a05113b.chromatic.com/?path=/docs/experimental-actionbar--docs
Checklist
Obligatory:
livechat/design-system
)