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

Match links support custom routing. #32

Closed ted537 closed 2 years ago

ted537 commented 2 years ago

Closes #31

When using a Single Page Application router such as React Router, the application code needs access to both the href prop and the onClick() prop.

Providing the underlying mouse event to the router lets it decide when and when not to override the browser's default behaviour. Providing the href prop lets the browser open a link in a new tab (when applicable) or copy the link.

Shenato commented 2 years ago

Sorry for the late response! Just saw the issues you opened and this PR, really like the simple solution and I think this is a good extension of functionality for how to handle match events.

One small question I have because i noticed the code isn't 100% linted. Does the current setup automatically allow your VSCode editor to format the code according to the project's lint rules or do you have to run the npm run lint command every time? I would like feedback just to see if there's something I can do to make the project's lint rules automatically apply for contributors without unnecessary friction.

Otherwise thanks alot for the contribution, I can merge it in as soon as you run the linter. 👍

ted537 commented 2 years ago

Hm, interesting. Here's my output from running the linter command, and I don't think any of these errors are caused by my changes.

ted@ted-MS-7817:~/git/react-tournament-brackets$ npm run lint

> @g-loot/react-tournament-brackets@1.0.0 lint
> eslint "src/**/*.js" "src/**/*.jsx"

/home/ted/git/react-tournament-brackets/src/bracket-custom-match.stories.jsx
  38:9  error  'match' is defined but never used           no-unused-vars
  39:9  error  'onMatchClick' is defined but never used    no-unused-vars
  40:9  error  'onPartyClick' is defined but never used    no-unused-vars
  42:9  error  'onMouseLeave' is defined but never used    no-unused-vars
  45:9  error  'topWon' is defined but never used          no-unused-vars
  46:9  error  'bottomWon' is defined but never used       no-unused-vars
  47:9  error  'topHovered' is defined but never used      no-unused-vars
  48:9  error  'bottomHovered' is defined but never used   no-unused-vars
  49:9  error  'topText' is defined but never used         no-unused-vars
  50:9  error  'bottomText' is defined but never used      no-unused-vars
  51:9  error  'connectorColor' is defined but never used  no-unused-vars
  52:9  error  'computedStyles' is defined but never used  no-unused-vars

/home/ted/git/react-tournament-brackets/src/bracket-single/single-elim-bracket.stories.jsx
  70:5  error  'match' is defined but never used           no-unused-vars
  71:5  error  'onMatchClick' is defined but never used    no-unused-vars
  72:5  error  'onPartyClick' is defined but never used    no-unused-vars
  73:5  error  'onMouseEnter' is defined but never used    no-unused-vars
  74:5  error  'onMouseLeave' is defined but never used    no-unused-vars
  75:5  error  'topParty' is defined but never used        no-unused-vars
  76:5  error  'bottomParty' is defined but never used     no-unused-vars
  77:5  error  'topWon' is defined but never used          no-unused-vars
  78:5  error  'bottomWon' is defined but never used       no-unused-vars
  79:5  error  'topHovered' is defined but never used      no-unused-vars
  80:5  error  'bottomHovered' is defined but never used   no-unused-vars
  81:5  error  'topText' is defined but never used         no-unused-vars
  82:5  error  'bottomText' is defined but never used      no-unused-vars
  83:5  error  'connectorColor' is defined but never used  no-unused-vars
  84:5  error  'computedStyles' is defined but never used  no-unused-vars

✖ 27 problems (27 errors, 0 warnings)
Shenato commented 2 years ago

And does project have correct settings to allow you to format the file if you just run "format document" in vscode (if you're using VSCode)

Shenato commented 2 years ago

My bad I believe you're supposed to run npm run format not lint. And yes you are correct none of those are caused by your change, they're incomplete examples in storybook.

ted537 commented 2 years ago

As far as I can tell, npm run format is the same as npm run lint, but applies changes rather than just detecting oddities.

https://github.com/g-loot/react-tournament-brackets/blob/08867818a605bd012ad165a22e8f848504251f53/package.json#L16-L17

I see there's a prettier config in the repository. Should that be used?

Result of applying prettier via npx prettier --write .

Example 1

Translates

    event: React.MouseEvent<HTMLAnchorElement,MouseEvent>;

to

    event: React.MouseEvent<HTMLAnchorElement, MouseEvent>;

Example 2

Translates

            onClick={(event) => onMatchClick?.({ match, topWon, bottomWon,event })}

to

            onClick={event =>
              onMatchClick?.({ match, topWon, bottomWon, event })
            }
Shenato commented 2 years ago

Perfect! if you push that I can merge this in.

I will have to look into why the project code-style rules aren't applying by default without having to run npm run format

Shenato commented 2 years ago

Yes the prettier config is what i was referring to!

ted537 commented 2 years ago

I believe the linter inconsistencies might be caused by the lint and format scripts only targeting .js and .jsx files, as they wouldn't pick up .ts and .tsx files.

Shenato commented 2 years ago

Good catch, I didn't realize that, I will update them.