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

Addon-contexts improvements: bug-fixing, testing, typing #6572

Merged
merged 7 commits into from
Apr 21, 2019

Conversation

leoyli
Copy link
Contributor

@leoyli leoyli commented Apr 21, 2019

Issue: #6559

What I did

  • [bug-fix] when a param in a context carrying props = null, it mistakenly disable the context.
  • [test] add 4 unit testing files to cover the most important logic of this addon (API related).
  • [type] improve typing and its maintainability (separate type and implementation but collocate them, greatly inspired by @apastuhov, thanks!).
  • [doc] fix typos in code comments, and remove the old NPM badge in README (migration leftovers).
  • [build] strip-out type comments in the build.

How to test

  • Running Jest (CI/CD should automate this).
  • Look into Addon|Contexts stories in the official storybook (should not break the stories).

UPDATE:

  • I have attached the above shortcut link and verify it did not break the addon stories.

@leoyli leoyli added bug cleanup Minor cleanup style change that won't show up in release changelog typescript addon: contexts labels Apr 21, 2019
@leoyli leoyli added this to the 5.1.0 milestone Apr 21, 2019
@leoyli leoyli self-assigned this Apr 21, 2019
@vercel
Copy link

vercel bot commented Apr 21, 2019

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

Latest deployment for this branch: https://monorepo-git-add-addon-contexts-tests.storybook.now.sh

@leoyli leoyli removed the cleanup Minor cleanup style change that won't show up in release changelog label Apr 21, 2019
@storybookjs storybookjs deleted a comment from github-actions bot Apr 21, 2019
@storybookjs storybookjs deleted a comment from github-actions bot Apr 21, 2019
@leoyli leoyli requested review from gaetanmaisse and kroeder April 21, 2019 03:46
@leoyli
Copy link
Contributor Author

leoyli commented Apr 21, 2019

@shilman... the automation bot seems a bit crazy here... Did I do something wrong or you think it could be a bug 😢...

@codecov
Copy link

codecov bot commented Apr 21, 2019

Codecov Report

Merging #6572 into next will increase coverage by 0.49%.
The diff coverage is 41.66%.

Impacted file tree graph

@@           Coverage Diff            @@
##            next   #6572      +/-   ##
========================================
+ Coverage   40.1%   40.6%   +0.49%     
========================================
  Files        633     633              
  Lines       8660    8660              
  Branches     619     620       +1     
========================================
+ Hits        3473    3516      +43     
+ Misses      5098    5055      -43     
  Partials      89      89
Impacted Files Coverage Δ
addons/contexts/src/preview/libs/decorators.ts 100% <ø> (ø)
addons/contexts/src/preview/index.ts 0% <ø> (ø) ⬆️
addons/contexts/src/manager/libs/useChannel.ts 0% <ø> (ø) ⬆️
...ddons/contexts/src/preview/libs/getContextNodes.ts 92.3% <ø> (+92.3%) ⬆️
addons/contexts/src/preview/frameworks/react.ts 0% <0%> (ø) ⬆️
addons/contexts/src/manager/ToolBarMenuOptions.tsx 0% <0%> (ø) ⬆️
addons/contexts/src/manager/ToolbarControl.tsx 0% <0%> (ø) ⬆️
addons/contexts/src/preview/frameworks/vue.ts 0% <0%> (ø) ⬆️
addons/contexts/src/manager/AddonManager.tsx 0% <0%> (ø) ⬆️
addons/contexts/src/manager/ToolBarMenu.tsx 0% <0%> (ø) ⬆️
... and 10 more

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 74582af...5100a4f. Read the comment docs.

@codecov
Copy link

codecov bot commented Apr 21, 2019

Codecov Report

Merging #6572 into next will increase coverage by 0.5%.
The diff coverage is 44%.

Impacted file tree graph

@@           Coverage Diff            @@
##            next    #6572     +/-   ##
========================================
+ Coverage   40.1%   40.61%   +0.5%     
========================================
  Files        633      633             
  Lines       8660     8660             
  Branches     619      620      +1     
========================================
+ Hits        3473     3517     +44     
+ Misses      5098     5054     -44     
  Partials      89       89
Impacted Files Coverage Δ
addons/contexts/src/preview/libs/decorators.ts 100% <ø> (ø)
addons/contexts/src/preview/index.ts 0% <ø> (ø) ⬆️
addons/contexts/src/manager/libs/useChannel.ts 0% <ø> (ø) ⬆️
addons/contexts/src/preview/frameworks/react.ts 0% <0%> (ø) ⬆️
addons/contexts/src/manager/ToolBarMenuOptions.tsx 0% <0%> (ø) ⬆️
addons/contexts/src/manager/ToolbarControl.tsx 0% <0%> (ø) ⬆️
addons/contexts/src/preview/frameworks/vue.ts 0% <0%> (ø) ⬆️
addons/contexts/src/manager/AddonManager.tsx 0% <0%> (ø) ⬆️
addons/contexts/src/manager/ToolBarMenu.tsx 0% <0%> (ø) ⬆️
addons/contexts/src/manager/ToolBar.tsx 0% <0%> (ø) ⬆️
... and 10 more

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 74582af...8caf900. Read the comment docs.

@shilman
Copy link
Member

shilman commented Apr 21, 2019

@leoyli It's because you tagged this typescript--I don't think this is really a typescript issue per se, so I removed it.

@leoyli leoyli merged commit e579414 into next Apr 21, 2019
@leoyli leoyli deleted the add/addon-contexts-tests branch April 21, 2019 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants