Skip to content
This repository has been archived by the owner on Dec 8, 2022. It is now read-only.

color picker accepts rgb format #1775

Conversation

Blackbaud-JackMcElhinney
Copy link
Contributor

Accepts rgb format like 'rgb(255, 0, 0)' as well as rgba(255, 0, 0, 1)

Resolves: #1620

@blackbaud-sky-savage
Copy link
Collaborator

Tests passed. Automated cross-browser testing via BrowserStack and Travis CI shows that the JavaScript changes in this pull request are: CONFIRMED

Commit: c753260
Build details: https://travis-ci.org/blackbaud-sky-savage/skyux2/builds/397953526

(Please note that this is a fully automated comment.)

@codecov-io
Copy link

codecov-io commented Jun 28, 2018

Codecov Report

Merging #1775 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1775   +/-   ##
=======================================
  Coverage   99.98%   99.98%           
=======================================
  Files         410      410           
  Lines        8585     8585           
  Branches     1266     1267    +1     
=======================================
  Hits         8584     8584           
  Misses          1        1
Impacted Files Coverage Δ
src/modules/colorpicker/colorpicker.service.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 a3935ea...c769e5d. Read the comment docs.

Copy link
Member

@Blackbaud-SteveBrush Blackbaud-SteveBrush left a comment

Choose a reason for hiding this comment

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

@Blackbaud-JackMcElhinney Mind writing a unit test to confirm this behavior?

@blackbaud-sky-savage
Copy link
Collaborator

Tests failed. Automated cross-browser testing via BrowserStack and Travis CI shows that the JavaScript changes in this pull request are: BUSTED

Commit: 4cd4b98
Build details: https://travis-ci.org/blackbaud-sky-savage/skyux2/builds/399735341

(Please note that this is a fully automated comment.)

@blackbaud-sky-savage
Copy link
Collaborator

Tests failed. Automated cross-browser testing via BrowserStack and Travis CI shows that the JavaScript changes in this pull request are: BUSTED

Commit: 3d59358
Build details: https://travis-ci.org/blackbaud-sky-savage/skyux2/builds/399761493

(Please note that this is a fully automated comment.)

@Blackbaud-SteveBrush
Copy link
Member

@Blackbaud-JackMcElhinney Thanks for the unit test. I would expect to see a rgb property set on the response object (without the alpha value). Here's what's currently returned from the colorpicker (I'm only seeing the rgba value):

screen shot 2018-07-17 at 9 48 38 am

Is this intended?

@blackbaud-sky-savage
Copy link
Collaborator

Tests failed. Automated cross-browser testing via BrowserStack and Travis CI shows that the JavaScript changes in this pull request are: BUSTED

Commit: f9d2143
Build details: https://travis-ci.org/blackbaud-sky-savage/skyux2/builds/404907497

(Please note that this is a fully automated comment.)

@Blackbaud-JackMcElhinney
Copy link
Contributor Author

@Blackbaud-SteveBrush That is intended based off of the way I made the change. I just made it so that if no alpha value was sent, it defaults it to 1. So even though the user sends rgb, it comes back with rgba where the alpha is 1. I did this so it already fits in with the outputFormat options of 'rgba', 'hsla', 'cmyk', and 'hex.' Let me know if you want me to look into adding 'rgb' as an output with no alpha.

@Blackbaud-SteveBrush
Copy link
Member

@blackbaud-sky-savage retry

@blackbaud-sky-savage
Copy link
Collaborator

Tests failed. Automated cross-browser testing via BrowserStack and Travis CI shows that the JavaScript changes in this pull request are: BUSTED

Commit: f9d2143
Build details: https://travis-ci.org/blackbaud-sky-savage/skyux2/builds/405001269

(Please note that this is a fully automated comment.)

@blackbaud-sky-savage
Copy link
Collaborator

Tests passed. Automated cross-browser testing via BrowserStack and Travis CI shows that the JavaScript changes in this pull request are: CONFIRMED

Commit: c769e5d
Build details: https://travis-ci.org/blackbaud-sky-savage/skyux2/builds/405039178

(Please note that this is a fully automated comment.)

@Blackbaud-SteveBrush Blackbaud-SteveBrush merged commit 37a540b into blackbaud:master Jul 17, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants