Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add TypeScript types for <Overlay>, <OverlayTrigger>, <Tooltip>, <IconButton> #3100

Merged
merged 5 commits into from
Jul 7, 2024

Conversation

bradenmacdonald
Copy link
Contributor

@bradenmacdonald bradenmacdonald commented Jun 17, 2024

Description

This adds types for <Overlay>, <OverlayTrigger>, <Tooltip>, and <IconButton> by converting their implementations from .jsx to .tsx.

This also fixes a number of random type errors that then showed up in the various test files, and makes some minor cleanups to <Chip> related code, such as removing // @ts-ignore directives now that we have typings available for <IconButton>.

Deploy Preview

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

Merge Checklist

n/a for this type of internal change.

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 openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Jun 17, 2024
@openedx-webhooks
Copy link

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.

Copy link

netlify bot commented Jun 17, 2024

Deploy Preview for paragon-openedx ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 4aee81f
🔍 Latest deploy log https://app.netlify.com/sites/paragon-openedx/deploys/66881ed9c07cc700084d9c5b
😎 Deploy Preview https://deploy-preview-3100--paragon-openedx.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

Copy link

codecov bot commented Jun 17, 2024

Codecov Report

Attention: Patch coverage is 94.11765% with 1 line in your changes missing coverage. Please review.

Project coverage is 93.25%. Comparing base (cdec543) to head (4aee81f).
Report is 23 commits behind head on master.

Files Patch % Lines
src/Overlay/index.tsx 83.33% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3100      +/-   ##
==========================================
+ Coverage   93.20%   93.25%   +0.04%     
==========================================
  Files         249      249              
  Lines        4359     4388      +29     
  Branches     1034     1037       +3     
==========================================
+ Hits         4063     4092      +29     
  Misses        290      290              
  Partials        6        6              

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

Copy link
Contributor

@brian-smith-tcril brian-smith-tcril left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is wonderful! I'm super happy to see components cleaned up like this!

LGTM!

@@ -37,7 +37,7 @@ const {

export type CollapsibleLiveEditorTypes = {
children: React.ReactNode;
clickToCopy: (arg: string) => void;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@adamstankiewicz do you know why arg: string was in here before?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most likely, we previously passed some kind of parameter here, but now this parameter does not exist.
Good catch! 💯

Copy link
Contributor

@PKulkoRaccoonGang PKulkoRaccoonGang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great, left some suggestions

src/Chip/ChipIcon.tsx Outdated Show resolved Hide resolved
src/Chip/ChipIcon.tsx Outdated Show resolved Hide resolved
@@ -37,7 +37,7 @@ const {

export type CollapsibleLiveEditorTypes = {
children: React.ReactNode;
clickToCopy: (arg: string) => void;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most likely, we previously passed some kind of parameter here, but now this parameter does not exist.
Good catch! 💯

@bradenmacdonald
Copy link
Contributor Author

@PKulkoRaccoonGang Thanks for the review! I made those suggested changes.

@bradenmacdonald
Copy link
Contributor Author

@brian-smith-tcril or @PKulkoRaccoonGang could one of you please merge this at your convenience? :)

@PKulkoRaccoonGang PKulkoRaccoonGang merged commit 8985c07 into openedx:master Jul 7, 2024
10 checks passed
@openedx-webhooks
Copy link

@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.

@PKulkoRaccoonGang
Copy link
Contributor

@bradenmacdonald 🚀

PKulkoRaccoonGang pushed a commit that referenced this pull request Jul 7, 2024
…, <IconButton> (#3100)

* feat: add typings for <Overlay> and <OverlayTrigger>

* feat: add typings for <Tooltip>

* feat: add typings for <IconButton>

* chore: typing cleanups for <Chip> code

* feat: slightly more detailed types for <Chip/ChipIcon> - per review
@bradenmacdonald bradenmacdonald deleted the braden/more-types branch July 8, 2024 09:03
@bradenmacdonald
Copy link
Contributor Author

Thanks @PKulkoRaccoonGang !

@openedx-semantic-release-bot

🎉 This PR is included in version 22.7.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

PKulkoRaccoonGang pushed a commit that referenced this pull request Jul 22, 2024
…, <IconButton> (#3100)

* feat: add typings for <Overlay> and <OverlayTrigger>

* feat: add typings for <Tooltip>

* feat: add typings for <IconButton>

* chore: typing cleanups for <Chip> code

* feat: slightly more detailed types for <Chip/ChipIcon> - per review
PKulkoRaccoonGang pushed a commit that referenced this pull request Aug 1, 2024
…, <IconButton> (#3100)

* feat: add typings for <Overlay> and <OverlayTrigger>

* feat: add typings for <Tooltip>

* feat: add typings for <IconButton>

* chore: typing cleanups for <Chip> code

* feat: slightly more detailed types for <Chip/ChipIcon> - per review
PKulkoRaccoonGang pushed a commit that referenced this pull request Aug 4, 2024
…, <IconButton> (#3100)

* feat: add typings for <Overlay> and <OverlayTrigger>

* feat: add typings for <Tooltip>

* feat: add typings for <IconButton>

* chore: typing cleanups for <Chip> code

* feat: slightly more detailed types for <Chip/ChipIcon> - per review
PKulkoRaccoonGang pushed a commit that referenced this pull request Aug 5, 2024
…, <IconButton> (#3100)

* feat: add typings for <Overlay> and <OverlayTrigger>

* feat: add typings for <Tooltip>

* feat: add typings for <IconButton>

* chore: typing cleanups for <Chip> code

* feat: slightly more detailed types for <Chip/ChipIcon> - per review
PKulkoRaccoonGang pushed a commit that referenced this pull request Aug 5, 2024
…, <IconButton> (#3100)

* feat: add typings for <Overlay> and <OverlayTrigger>

* feat: add typings for <Tooltip>

* feat: add typings for <IconButton>

* chore: typing cleanups for <Chip> code

* feat: slightly more detailed types for <Chip/ChipIcon> - per review
PKulkoRaccoonGang pushed a commit that referenced this pull request Nov 26, 2024
…, <IconButton> (#3100)

* feat: add typings for <Overlay> and <OverlayTrigger>

* feat: add typings for <Tooltip>

* feat: add typings for <IconButton>

* chore: typing cleanups for <Chip> code

* feat: slightly more detailed types for <Chip/ChipIcon> - per review
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
open-source-contribution PR author is not from Axim or 2U released
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants