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

Update MUI and take some care of Storybook #4115

Merged
merged 24 commits into from
May 16, 2019
Merged

Update MUI and take some care of Storybook #4115

merged 24 commits into from
May 16, 2019

Conversation

dominik-zeglen
Copy link
Contributor

@dominik-zeglen dominik-zeglen commented May 15, 2019

I want to merge this change because it adds new inputs and improves storybook a bit.

Screenshots

Screenshot 2019-05-15 at 15 09 41

Screenshot 2019-05-15 at 15 09 52

Screenshot 2019-05-15 at 15 10 05

Pull Request Checklist

  1. Privileged views and APIs are guarded by proper permission checks.
  2. All visible strings are translated with proper context.
  3. All data-formatting is locale-aware (dates, numbers, and so on).
  4. Database queries are optimized and the number of queries is constant.
  5. Database migration files are up to date.
  6. The changes are tested.
  7. The code is documented (docstrings, project documentation).
  8. GraphQL schema and type definitions are up to date.
  9. Changes are mentioned in the changelog.

@piotrgrundas
Copy link
Contributor

piotrgrundas commented May 15, 2019

image
image

Feature branche please!

@codecov
Copy link

codecov bot commented May 15, 2019

Codecov Report

Merging #4115 into master will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4115      +/-   ##
==========================================
+ Coverage   91.51%   91.52%   +0.01%     
==========================================
  Files         277      277              
  Lines       15083    15083              
  Branches     1479     1479              
==========================================
+ Hits        13803    13805       +2     
+ Misses        875      874       -1     
+ Partials      405      404       -1
Impacted Files Coverage Δ
saleor/payment/utils.py 91.5% <0%> (+0.94%) ⬆️

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 d212e1d...69fa94b. Read the comment docs.

@dominik-zeglen
Copy link
Contributor Author

Oh cmon it's mostly snapshots!

@piotrgrundas
Copy link
Contributor

It doesn't really mater what it is. U know that equally I can just approve it because of amount of changes....

@dominik-zeglen
Copy link
Contributor Author

There's like 500 LOC that really do need to be read. The rest are snapshots and one-line changes like
variant="headline" -> variant="h5".

Copy link
Member

@patrys patrys left a comment

Choose a reason for hiding this comment

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

We should not be using Checkbox component's onClick property before and I don't think we should be using it now. Please at least implement an onChange property so we can gradually fix our code.

@maarcingebala maarcingebala merged commit c6612aa into master May 16, 2019
patrys
patrys approved these changes May 16, 2019
@maarcingebala maarcingebala deleted the update-mui branch May 16, 2019 12:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants