search

zakodium-oss / react-science

React components and tools to build scientific applications.
https://react-science.pages.dev
MIT License
2 stars 6 forks source link

Implement Panel Toolbar #735

Closed wadjih-bencheikh18 closed 2 days ago

wadjih-bencheikh18 commented 3 months ago

BEGIN_COMMIT_OVERRIDE feat: add minimal and fill props to toolbar component and improve style

BREAKING CHANGE: the new minimal prop must be set to true in order to keep a similar appearance when upgrading.

END_COMMIT_OVERRIDE

cloudflare-workers-and-pages[bot] commented 3 months ago

Deploying react-science with  Cloudflare Pages  Cloudflare Pages

Latest commit: 634df96
Status: ✅  Deploy successful!
Preview URL: https://5c30b092.react-science.pages.dev
Branch Preview URL: https://243-create-toolwindow-compon.react-science.pages.dev

View logs

wadjih-bencheikh18 commented 2 months ago

@stropitek this PR is ready to merge

wadjih-bencheikh18 commented 2 months ago

Can you just focus on a simple stateless component that renders the big toolbar buttons and remove the rest of the code?

Is it worth to create a component for that? isn't it enough to create just story for it. because the implementation is simply div with toolbar items inside.

stropitek commented 2 months ago

Is it worth to create a component for that? isn't it enough to create just story for it. because the implementation is simply div with toolbar items inside.

Indeed if we don't need custom style and just layout with existing components then a story is enough.

I would suggest that the story has some state which changes the buttons from active to inactive when you click them.

wadjih-bencheikh18 commented 1 week ago

Please review any missing controls (like minimal and vertical) in the control story and add them to the story.

Improved

wadjih-bencheikh18 commented 4 days ago

It would make sense that the large version of the component also shows the larger icons, no?

Yes. Blueprint Button use the small icon in all cases https://blueprintjs.com/docs/#core/components/buttons

wadjih-bencheikh18 commented 4 days ago

Can you do that so that we can see if it looks better?

You can take a look. For the toolbar icons they can be string (blueprint native icons) or component or text I tried to check all these cases

wadjih-bencheikh18 commented 3 days ago

I noticed that when you use the vertical version with the fill option to true, it fills the horizontal space instead of the vertical space. Can you check if it can be changed to be like https://blueprintjs.com/docs/#core/components/button-group ?

It's working now

wadjih-bencheikh18 commented 2 days ago

Do you see any breaking changes that need to be documented?

Only changing the default minimal in toolbar which I added a commit for it