Skip to content
This repository was archived by the owner on Jan 13, 2025. It is now read-only.

feat(icon-button): Convert to TypeScript #4325

Merged
merged 18 commits into from
Feb 6, 2019

Conversation

abhiomkar
Copy link
Collaborator

@abhiomkar abhiomkar commented Jan 30, 2019

Refs #4225

@moog16
Copy link
Contributor

moog16 commented Jan 31, 2019

Don't forget to also update js-bundle-factory.js to

iconButton: getAbsolutePath('/packages/mdc-icon-button/index.ts'),

@kfranqueiro kfranqueiro mentioned this pull request Jan 31, 2019
45 tasks
@kfranqueiro
Copy link
Contributor

Do we also need to update index to index.ts for icon-button in material-components-web/index.js? It looks like we've been doing that for the others as they've been converted.

@moog16
Copy link
Contributor

moog16 commented Jan 31, 2019

@kfranquiero I think we may have missed a few component declarations in material-components-web/index.js. We can do it here to be consistent, but we will eventually remove all the .ts exts anyways during the clean up phase of #4225.

@kfranqueiro
Copy link
Contributor

@moog16 afaict we have done it for every package that's been hit so far, with the exception of animation which doesn't appear in material-components-web/index.js at all. I was assuming it was necessary for the all-in-one build to work?

@moog16
Copy link
Contributor

moog16 commented Jan 31, 2019

Don't think it is necessary since webpack resolves either .js or .ts.

I would test...but waiting for npm i to finish. :|

@moog16
Copy link
Contributor

moog16 commented Feb 6, 2019

ensure that the all-in-one package compiles without adding the .ts extensions

@codecov-io
Copy link

codecov-io commented Feb 6, 2019

Codecov Report

Merging #4325 into feat/typescript will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@                 Coverage Diff                 @@
##           feat/typescript    #4325      +/-   ##
===================================================
+ Coverage            98.39%   98.39%   +<.01%     
===================================================
  Files                   93       93              
  Lines                 5793     5803      +10     
  Branches               782      784       +2     
===================================================
+ Hits                  5700     5710      +10     
  Misses                  92       92              
  Partials                 1        1
Impacted Files Coverage Δ
packages/mdc-icon-button/index.ts 100% <100%> (ø)
packages/mdc-icon-button/foundation.ts 100% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0f354ae...fe14419. Read the comment docs.

@mdc-web-bot
Copy link
Collaborator

All 621 screenshot tests passed for commit 3f76537 vs. feat/typescript! 💯🎉

@mdc-web-bot
Copy link
Collaborator

All 621 screenshot tests passed for commit b7aed18 vs. feat/typescript! 💯🎉

@mdc-web-bot
Copy link
Collaborator

All 621 screenshot tests passed for commit 213088c vs. feat/typescript! 💯🎉

@mdc-web-bot
Copy link
Collaborator

All 621 screenshot tests passed for commit 8f022e3 vs. feat/typescript! 💯🎉

@mdc-web-bot
Copy link
Collaborator

All 621 screenshot tests passed for commit 8c460ec vs. feat/typescript! 💯🎉

@mdc-web-bot
Copy link
Collaborator

All 621 screenshot tests passed for commit dd61d5a vs. feat/typescript! 💯🎉

@acdvorak acdvorak self-assigned this Feb 6, 2019
Copy link
Contributor

@acdvorak acdvorak left a comment

Choose a reason for hiding this comment

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

LGTM!

@mdc-web-bot
Copy link
Collaborator

All 621 screenshot tests passed for commit fe14419 vs. feat/typescript! 💯🎉

@acdvorak acdvorak merged commit f595a99 into feat/typescript Feb 6, 2019
@acdvorak acdvorak deleted the feat/typescript_icon_button branch February 6, 2019 04:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants