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

feat(ActionBar): logic refactor #1219

Merged
merged 6 commits into from
Jul 4, 2024
Merged

Conversation

marcinsawicki
Copy link
Contributor

@marcinsawicki marcinsawicki commented Jun 28, 2024

Resolves: #1214

Description

Logic refactor:

  • changed IntersectionObserver to ResizeObserver - from now logic will observe the container size changes, then will calculate how many option buttons will fit in the currently available space:
    • nothing is changing with buttons possible to show in the container
    • buttons for which there is not enough space will not be rendered (that was necessary only for IntersectionObserver) and will be available in the menu
  • menu will be placed relative to the other elements (no more absolute positioning)

Storybook

https://feature-action-bar-menu-icon-fix--613a8e945a5665003a05113b.chromatic.com

Checklist

Obligatory:

  • Self review (use this as your final check for proposed changes before requesting the review)
  • Add reviewers (livechat/design-system)
  • Add correct label
  • Assign pull request with the correct issue

@marcinsawicki marcinsawicki added the bug Something isn't working label Jun 28, 2024
@marcinsawicki marcinsawicki added this to the Cycle #8 milestone Jun 28, 2024
@marcinsawicki marcinsawicki self-assigned this Jun 28, 2024
@marcinsawicki marcinsawicki marked this pull request as ready for review June 28, 2024 11:36
Copy link

@vladko-uxds vladko-uxds left a comment

Choose a reason for hiding this comment

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

👍

@marcinsawicki marcinsawicki linked an issue Jun 28, 2024 that may be closed by this pull request
Copy link
Contributor

@VadymBezpalko VadymBezpalko left a comment

Choose a reason for hiding this comment

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

👍
P.S.: Have you checked the component with React profiler to see how it renders?

threshold: 1,
};
const shouldDisplayMenu = !isScrollType && menuItemsKeys.length !== 0;
const singleElementSize = 44;
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't it be available as a prop? If not I'd suggest to move it outside the component's function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently not, but in the future I plan to improve that component and give a possibility to set the buttons size.

const observer = new ResizeObserver((entries) => {
const { width, height } = entries[0].contentRect;
const containerSize = vertical ? height : width;
const exstraSpacing = menuOptions.length > 0 ? 60 : 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

extraSpacing


return filteredOptions.map((o) => {
const getMenuItems = (menuOptions: IActionBarOption[]) =>
Copy link
Contributor

Choose a reason for hiding this comment

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

useCallback?

@VadymBezpalko VadymBezpalko merged commit f04e250 into main Jul 4, 2024
5 checks passed
@VadymBezpalko VadymBezpalko deleted the feature/action-bar-menu-icon-fix branch July 4, 2024 10:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

[ActionBar] - bug with last icon
3 participants