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

[improve: code coverage] ColorControl.js #81

Conversation

satotake
Copy link
Contributor

@satotake satotake commented Jul 9, 2017

related: https://github.com/times/cardkit/issues/61

Basically, this PR just added tests to ColorControl.js based on its original implementation,
I mean, it never adds any feature or functionality and is tiny improvement about coverage.
There is no real reason for starting with ColorControl.

Anyway...
Writing tests of ColorControl gave me 3 ideas.

  1. it may be better for ColorControl to have a validator for layer prop.
    • ColorControl virtually requires layer.editable property but it is implicative.
  2. Considered about the previous idea, it may be better for layer.editable to have consistency in terms of type.
    • For now, layer.editable[name] accepts object, string and boolean and it defines DOM content of ColorControl. I suspect this is complicated.
    • Just an idea
      • editable: {picker: 'circle', options: [...]} => CirclePicker
      • editable: {picker: 'chrome'} => ChromePicker
      • editable: {picker: 'input'} => input tag
  3. ColorControl#handleChange should validate an argument.
    • Because it may not be a color-associated string.

If you are favor of even one of the ideas, I am willing to implement it (them) in next PR.
Thanks for reading.

@satotake satotake changed the title [improve: code coverage [improve: code coverage] ColorControl.js Jul 9, 2017
@codecov-io
Copy link

Codecov Report

Merging #81 into master will increase coverage by 2.45%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #81      +/-   ##
==========================================
+ Coverage   41.67%   44.13%   +2.45%     
==========================================
  Files          25       25              
  Lines        1310     1312       +2     
  Branches      250      251       +1     
==========================================
+ Hits          546      579      +33     
+ Misses        764      733      -31
Impacted Files Coverage Δ
.../renderers/dom/ui/elements/Controls/SizeControl.js 34.88% <ø> (ø) ⬆️
...renderers/dom/ui/elements/Controls/ColorControl.js 100% <100%> (+60.86%) ⬆️
...nderers/dom/ui/elements/Panels/Layout/component.js 36.58% <0%> (ø) ⬆️
...rs/dom/ui/elements/Panels/Template/CardTemplate.js 37.93% <0%> (ø) ⬆️
src/renderers/shared/Card.js 13.52% <0%> (ø) ⬆️
...enderers/dom/ui/elements/Controls/SourceControl.js 22.8% <0%> (ø) ⬆️
...c/renderers/dom/ui/elements/Sidebar/PanelButton.js 42.85% <0%> (ø) ⬆️
src/renderers/dom/ui/elements/Canvas/component.js 65.21% <0%> (ø) ⬆️
src/renderers/dom/ui/ui.js 29.5% <0%> (ø) ⬆️
...erers/dom/ui/elements/Panels/Template/component.js 27.27% <0%> (ø) ⬆️
... and 8 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 98a1a62...1d9b09f. Read the comment docs.

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.

None yet

3 participants