search

g-loot / react-tournament-brackets

React component library for displaying bracket leaderboards
https://sleepy-kare-d8538d.netlify.app/?path=/story/components-bracket--bracket
GNU Lesser General Public License v2.1
219 stars 69 forks source link

Add ability to enable customToolbar #28

Closed hichemfantar closed 1 year ago

hichemfantar commented 2 years ago

closes #20 and #22

hichemfantar commented 2 years ago

Does this mean I'm forced to create a custom toolbar when I want to activate it? What if the developer is satisfied with the default toolbar and simply wants to activate it? I believe the option to use the default toolbar should be possible. I believe this PR is a breaking change considering it changes the default behavior of the library. Perhaps a simple major version bump alone with this PR is enough. Would love to hear your opinion on this.

Shenato commented 2 years ago

Does this mean I'm forced to create a custom toolbar when I want to activate it? What if the developer is satisfied with the default toolbar and simply wants to activate it?

In that case the developer can use the peerDependency react-svg-pan-zoom directly and write their own SvgViewer. this library's focus is the bracket component and the SvgViewer is meant to be a minimal solution to rendering it without any bloat.

I believe the option to use the default toolbar should be possible.

I agree with you, however I don't believe it should be the default option, and I don't believe this library should concern itself with solving all the possible user experience scenarios for the SvgViewer.

As I've said earlier the SvgViewer provided with the library aims to be an opinionated minimalistic viewer with sensible defaults to allow it to fit most use-cases, the aim is to serve most developers who don't want a default toolbar that clashes with their own design language when they implement this library in their UI.

If you like the default react-svg-pan-zoom's toolbar and you do not agree with this opinion, you can write your own simple viewer and use that.

Shenato commented 2 years ago

I've implemented the suggested solution in this PR #39

Shenato commented 1 year ago

Closing this pull request as I've implemented this in the other PR #39. Feel free to re-open the discussion if that's not what you were looking for.