Add Toolbar.PopoverItem

[moonayyur]

closes #612

[cloudflare-workers-and-pages]

Deploying with    Cloudflare Pages

Latest commit:	57bb079
Status:	✅  Deploy successful!
Preview URL:	https://6b9f3f8a.react-science.pages.dev
Branch Preview URL:	https://612-add-toolbarmenuitem-or-s.react-science.pages.dev

View logs

[stropitek]

Can you make a dedicated story with toolbar popover items? The feature is hard to discover in the stories.

Also if you have ideas on how we can make a popover item distinguishable from a regular item, that would be welcome. For example in photoshop there is this little triangular shape in the corner of the button:

[codecov-commenter]

Codecov Report

Attention: 181 lines in your changes are missing coverage. Please review.

Comparison is base (bf1bfde) 24.26% compared to head (b57c6b9) 23.39%.
Report is 4 commits behind head on main.

Files	Patch %	Lines
stories/components/toolbar.stories.tsx	0.00%	143 Missing and 1 partial ⚠️
src/components/toolbar/Toolbar.tsx	28.84%	37 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #635      +/-   ##
==========================================
- Coverage   24.26%   23.39%   -0.87%     
==========================================
  Files         223      221       -2     
  Lines       12903    12778     -125     
  Branches      234      234              
==========================================
- Hits         3131     2990     -141     
- Misses       9684     9700      +16     
  Partials       88       88              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[moonayyur]

@stropitek I added a right icon to the popover items like in the dropdown component. The icon has different directions when the toolbar is vertical or horizontal

[moonayyur]

@stropitek There is a bug with the vertical control in the Toolbar :
https://react-science.pages.dev/stories/?path=/story/components-toolbar--control

By default vertical has the value 'false' and the toolbar is horizontal, when we toggle the button the toolbar becomes vertical, but when we click on it again it doesn't go back to horizontal

[stropitek]

Can you also make a separate story where we mix dropdown items and non-dropdown items? Try to put them at the beginning, at the end and in between.

There are alignment issues as well in the vertical version of the toolbar with dropdown items.

By default vertical has the value 'false' and the toolbar is horizontal, when we toggle the button the toolbar becomes vertical, but when we click on it again it doesn't go back to horizontal

Do you need help with debugging this?

[moonayyur]

Do you need help with debugging this?

It is due to this code in the toolbar width recomputing :

if (!vertical) {
      return;
    }

Removing it fixes the problem

[stropitek]

@moonayyur

I changed to approach to fixing the issue with changing the vertical prop to be more conservative in the behaviour. The effect is not supposed to be executed in horizontal mode since the layout bug we are working around is only for the vertical layout.

I also fixed the issue with the icon alignment issue.

LGTM after you resolve the conversations.

@hamed-musallam can you have a look at the stories? What do you think of the toolbar item popover? Is it a good match for NMRium?

[stropitek]

ping @moonayyur @hamed-musallam

[stropitek]

I'm merging this, but I think there is too much layout difference between the dropdown and the non-dropdown item. The addition of the icon on the right doubles the size of the component.

I'm creating a new issue for that.

[hamed-musallam]

@stropitek

I believe it's essential to maintain consistent layout, width, and height for both dropdown and non-dropdown menus. Additionally, I suggest that toolbar items should not be highlighted upon clicking, as I only intend to open the dropdown menu.

[hamed-musallam]

On the other hand, I believe the popup animation is slow, can you increase the speed of the animation to have smooth animation?

[hamed-musallam]

it is just a suggestion, can we specify the trigger action as a long click, hover, or click on the dropdown toolbar.

[stropitek]

On the other hand, I believe the popup animation is slow, can you increase the speed of the animation to have smooth animation?

Those are in the CSS of blueprint and so are not editable. But we can use the minimal prop of popover which makes the animation much more subtle. It will also remove the "arrow" that points from the menu to the button.

it is just a suggestion, can we specify the trigger action as a long click, hover, or click on the dropdown toolbar.

I'd be against "long click" since this is difficult to discover and non-standard. What are you using in NMRium? All of those?
In the Popover component of blueprint it's possible to use click or hover

https://blueprintjs.com/docs/#core/components/popover

[hamed-musallam]

I'd be against "long click" since this is difficult to discover and non-standard. What are you using in NMRium? All of those? In the Popover component of blueprint it's possible to use click or hover
over

It's just a suggestion, but currently in NMRIUM, we only have a dropdown menu triggered by a click. What I propose is similar to Photoshop's functionality, where tools can be grouped and it is activated by long press

[stropitek]

I don't have a photoshop license but AFAIK it's because clicking selects the default tool in the category, thus the long click to select other related tools. Is that what you'd like for NMRIum.

I'm still skeptical about the long-press. Haven't seen this anywhere in the web I think.

[targos]

The web version of Photoshop doesn't have this behavior. They open the submenus on hover.

[stropitek]

@hamed-musallam actually I was wrong the Popover component has a property transitionDuration. You can set this property on the Toolbar.PopoverItem component.

[hamed-musallam]

thank you @stropitek , i will try it once I update to the latest version of react-science