feat: add typings for <Button> #3080

bradenmacdonald · 2024-05-22T18:19:19Z

Description

This adds typescript types for <Button> as well as <ButtonGroup> and <ButtonToolbar>.

I tested it against various usages of <Button> in frontend-app-course authoring, as well as all the usages in this repo itself, and couldn't find any issues.

Deploy Preview

https://deploy-preview-3080--paragon-openedx.netlify.app

Merge Checklist

If your update includes visual changes, have they been reviewed by a designer? Send them a link to the Netlify deploy preview, if applicable.
Does your change adhere to the documented style conventions?
Do any prop types have missing descriptions in the Props API tables in the documentation site (check deploy preview)?
Were your changes tested using all available themes (see theme switcher in the header of the deploy preview, under the "Settings" icon)?
Were your changes tested in the example app?
Is there adequate test coverage for your changes?
Consider whether this change needs to reviewed/QA'ed for accessibility (a11y). If so, please request an a11y review for the PR in the #wg-paragon Open edX Slack channel.

Post-merge Checklist

Verify your changes were released to NPM at the expected version.
If you'd like, share your contribution in #show-and-tell.
🎉 🙌 Celebrate! Thanks for your contribution.

openedx-webhooks · 2024-05-22T18:19:24Z

Thanks for the pull request, @bradenmacdonald! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

supporting documentation
Open edX discussion forum threads
timeline information ("this must be merged by XX date", and why that is)
partner information ("this is a course on edx.org")
any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

netlify · 2024-05-22T18:19:36Z

✅ Deploy Preview for paragon-openedx ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	`66f85cb`
🔍 Latest deploy log	https://app.netlify.com/sites/paragon-openedx/deploys/665779cd5beec7000806755b
😎 Deploy Preview	https://deploy-preview-3080--paragon-openedx.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

codecov · 2024-05-22T18:30:19Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.20%. Comparing base (48b8635) to head (66f85cb).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3080      +/-   ##
==========================================
+ Coverage   93.19%   93.20%   +0.01%     
==========================================
  Files         249      249              
  Lines        4348     4359      +11     
  Branches     1000     1036      +36     
==========================================
+ Hits         4052     4063      +11     
+ Misses        293      290       -3     
- Partials        3        6       +3

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

bradenmacdonald · 2024-05-27T21:57:47Z

src/Button/Button.test.tsx

-        render(<Button as={Hyperlink} destination="https://www.poop.com/💩">Button</Button>);
+        // eslint-disable-next-line @typescript-eslint/no-unused-vars
+        const ref = (_current: HTMLAnchorElement) => {}; // Check typing of a ref - should not show type errors.
+        render(<Button as={Hyperlink} ref={ref} destination="https://www.poop.com/💩">Button</Button>);


This shows an example of how passing as={Hyperlink} changes the types of various other props like ref to match.

bradenmacdonald · 2024-05-27T21:58:14Z

src/Button/Button.test.tsx

@@ -30,7 +30,7 @@ describe('<Button />', () => {

    it('renders with props iconAfter and size', () => {
      const tree = renderer.create((
-        <Button iconAfter={Close} size="md">Button</Button>
+        <Button iconAfter={Close} size="sm">Button</Button>


I just changed this to sm for some variation - too many of the tests were just using the same md size.

adamstankiewicz · 2024-05-29T15:40:07Z

src/Button/index.tsx

+import BaseButton, { type ButtonProps as BaseButtonProps } from 'react-bootstrap/Button';
+import BaseButtonGroup, { type ButtonGroupProps as BaseButtonGroupProps } from 'react-bootstrap/ButtonGroup';
+import BaseButtonToolbar, { type ButtonToolbarProps } from 'react-bootstrap/ButtonToolbar';
+import { type BsPrefixRefForwardingComponent as ComponentWithAsProp } from 'react-bootstrap/esm/helpers';


[question] Given this pattern of importing BsPrefixRefForwardingComponent (and BsPrefixProps, see #3082), I wonder if there's a more common/global location to define these types when renaming them so each individual component doesn't need to do the same re-naming repeatedly (e.g., risking different naming across components).

For example, Hyperlink (in #3082) will also rely on this pattern so it'd be nice to only rename it in one place.

Yes, that's a good idea and I'm happy to move it somewhere shared. Do you have a place in mind or want me to make something up?

First thing that came to my mind was to rename https://github.com/openedx/paragon/tree/master/src/utils/propTypes to src/utils/types and then put it in there, but I'm very open to any other ideas.

OK, I didn't rename propTypes for now since I want to distinguish them from types (but feel free to do that later if you want). But otherwise I did this: 66f85cb

I took the opportunity to add lots of documentation to explain how these types work (since the Bootstrap types are pretty obscure), and added in some type-only tests to verify that my understanding of them is correct (and support any future refactors).

brian-smith-tcril

This is wonderful! Thank you!

brian-smith-tcril · 2024-05-31T17:26:17Z

I believe codecov is stuck, I'm going to merge this now instead of waiting for that check

openedx-webhooks · 2024-05-31T17:26:25Z

@bradenmacdonald 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

openedx-semantic-release-bot · 2024-05-31T17:36:17Z

🎉 This PR is included in version 22.6.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

bradenmacdonald · 2024-05-31T18:38:32Z

Thanks @brian-smith-tcril !

* feat: add typings for <Button> * chore: bump @types/react and @types/react-dom * feat: re-export bootstrap helpers as ComponentWithAsProp, BsPropsWithAs

feat: add typings for <Button>

1c7c8d3

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label May 22, 2024

bradenmacdonald marked this pull request as draft May 22, 2024 18:19

chore: bump @types/react and @types/react-dom

1637767

bradenmacdonald marked this pull request as ready for review May 27, 2024 21:55

bradenmacdonald commented May 27, 2024

View reviewed changes

bradenmacdonald requested review from adamstankiewicz and brian-smith-tcril May 27, 2024 21:59

adamstankiewicz mentioned this pull request May 29, 2024

feat: introduce as prop for Hyperlink #3082

Draft

10 tasks

adamstankiewicz reviewed May 29, 2024

View reviewed changes

feat: re-export bootstrap helpers as ComponentWithAsProp, BsPropsWithAs

66f85cb

bradenmacdonald force-pushed the type-button branch from 23a1891 to 66f85cb Compare May 29, 2024 18:54

brian-smith-tcril approved these changes May 29, 2024

View reviewed changes

brian-smith-tcril merged commit 4abe68b into openedx:master May 31, 2024
9 of 10 checks passed

openedx-semantic-release-bot added the released label May 31, 2024

bradenmacdonald deleted the type-button branch May 31, 2024 18:38

PKulkoRaccoonGang pushed a commit that referenced this pull request Jul 7, 2024

feat: add typings for <Button> (#3080)

c81bc4b

* feat: add typings for <Button> * chore: bump @types/react and @types/react-dom * feat: re-export bootstrap helpers as ComponentWithAsProp, BsPropsWithAs

PKulkoRaccoonGang pushed a commit that referenced this pull request Jul 22, 2024

feat: add typings for <Button> (#3080)

90246aa

* feat: add typings for <Button> * chore: bump @types/react and @types/react-dom * feat: re-export bootstrap helpers as ComponentWithAsProp, BsPropsWithAs

PKulkoRaccoonGang pushed a commit that referenced this pull request Aug 5, 2024

feat: add typings for <Button> (#3080)

1fb7cd3

* feat: add typings for <Button> * chore: bump @types/react and @types/react-dom * feat: re-export bootstrap helpers as ComponentWithAsProp, BsPropsWithAs

PKulkoRaccoonGang pushed a commit that referenced this pull request Aug 5, 2024

feat: add typings for <Button> (#3080)

3f69e2e

* feat: add typings for <Button> * chore: bump @types/react and @types/react-dom * feat: re-export bootstrap helpers as ComponentWithAsProp, BsPropsWithAs

PKulkoRaccoonGang pushed a commit that referenced this pull request Nov 26, 2024

feat: add typings for <Button> (#3080)

98fe4a5

* feat: add typings for <Button> * chore: bump @types/react and @types/react-dom * feat: re-export bootstrap helpers as ComponentWithAsProp, BsPropsWithAs

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add typings for <Button> #3080

feat: add typings for <Button> #3080

bradenmacdonald commented May 22, 2024 •

edited

Loading

openedx-webhooks commented May 22, 2024 •

edited

Loading

netlify bot commented May 22, 2024 •

edited

Loading

codecov bot commented May 22, 2024 •

edited

Loading

bradenmacdonald May 27, 2024

bradenmacdonald May 27, 2024

adamstankiewicz May 29, 2024 •

edited

Loading

bradenmacdonald May 29, 2024

brian-smith-tcril May 29, 2024

bradenmacdonald May 29, 2024

brian-smith-tcril left a comment

brian-smith-tcril commented May 31, 2024

openedx-webhooks commented May 31, 2024

openedx-semantic-release-bot commented May 31, 2024

bradenmacdonald commented May 31, 2024

feat: add typings for <Button> #3080

feat: add typings for <Button> #3080

Conversation

bradenmacdonald commented May 22, 2024 • edited Loading

Description

Deploy Preview

Merge Checklist

Post-merge Checklist

openedx-webhooks commented May 22, 2024 • edited Loading

netlify bot commented May 22, 2024 • edited Loading

✅ Deploy Preview for paragon-openedx ready!

codecov bot commented May 22, 2024 • edited Loading

Codecov Report

bradenmacdonald May 27, 2024

Choose a reason for hiding this comment

bradenmacdonald May 27, 2024

Choose a reason for hiding this comment

adamstankiewicz May 29, 2024 • edited Loading

Choose a reason for hiding this comment

bradenmacdonald May 29, 2024

Choose a reason for hiding this comment

brian-smith-tcril May 29, 2024

Choose a reason for hiding this comment

bradenmacdonald May 29, 2024

Choose a reason for hiding this comment

brian-smith-tcril left a comment

Choose a reason for hiding this comment

brian-smith-tcril commented May 31, 2024

openedx-webhooks commented May 31, 2024

openedx-semantic-release-bot commented May 31, 2024

bradenmacdonald commented May 31, 2024

bradenmacdonald commented May 22, 2024 •

edited

Loading

openedx-webhooks commented May 22, 2024 •

edited

Loading

netlify bot commented May 22, 2024 •

edited

Loading

codecov bot commented May 22, 2024 •

edited

Loading

adamstankiewicz May 29, 2024 •

edited

Loading