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.

Sign up for GitHub

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

Jump to bottom

Enable alpha channel in color selector #9224

Merged

rijkvanzanten merged 263 commits into directus:main from adanielyan:color-with-alpha

Dec 31, 2021

Contributor

adanielyan commented Oct 29, 2021

This pull request adds alpha channel to Color interface in a form of 4th text box.

adanielyan requested a review from rijkvanzanten as a code owner

October 29, 2021 01:50

github-actions bot assigned adanielyan

azrikahar added App 🍰 Improvement labels

Sponsor Member

benhaynes commented Oct 29, 2021

Two questions: is RGB (without alpha) still available separately? And, can you post a screenshot of this in the smallest half width field size? We need to make sure this fits instead of wrapping oddly.

Thanks!! ❤️

Contributor Author

adanielyan commented Oct 29, 2021

Thank you for reviewing this PR.
The text boxes seem to fit nicely even on smallest screen.

When the alpha channel is FF (or 255) the color turns into RGB and the last 2 digits of hex code are removed (see the screenshot below). Therefore I believe having 2 separate variations in the dropdown (one for RGB and one for RGBA) would be redundant.

I changed the RGBA and HSLA labels to RGB(A) and HSL(A) respectively, to make this transformation less confusing.

Also bumped the max value for alpha channel to 255 to cover the entire spectrum (for some reason I initially decided to have percentage based range 0-100) in the last commit.

Please let me know if you have any other questions or concerns.

Sponsor Member

benhaynes commented Oct 29, 2021

My understanding is that alpha is commonly set as 0 to 100 and stored as rgba(r, g, b, a), though I realize there is also this hex spec. I feel that we should avoid confusion for the 90% of people who just want RGB by adding this as an interface option: Include Alpha that toggles this feature. @rijkvanzanten ?

Contributor Author

adanielyan commented Oct 29, 2021

OK, I see your point. How about this?

Screen.Recording.2021-10-29.at.12.36.54.mov

Contributor

jamescammarano commented Oct 29, 2021

should the bar be labeled? I wouldn't know what that was right now.

Contributor Author

adanielyan commented Oct 29, 2021 •

edited

should the bar be labeled? I wouldn't know what that was right now.

It could be, not sure where it will be the best to put the label though. On the other hand, it should not take a lot of effort to figure out what this slider does, even without a label.

EDIT: Also, adding a label will require providing translations too...

Contributor Author

adanielyan commented Oct 29, 2021

Does this look better?

Contributor

jamescammarano commented Oct 29, 2021

Does this look better?

I like that.

paulboudewijn and others added 18 commits

October 29, 2021 14:22


          Fix localstorage file deletion (directus#9126)

78ba2b9

* excluded VS specific files

* new files after updating directus sources

* case insensitive string matching fix

* Remove accidental commit files

* Cleanup gitignore

Co-authored-by: rijkvanzanten <rijkvanzanten@me.com>


          Update dependency vite to v2.6.11 (directus#9121)

e2a398b

Co-authored-by: Renovate Bot <bot@renovateapp.com>


          Update dependency axios to v0.24.0 (directus#9132)

fb173f2

Co-authored-by: Renovate Bot <bot@renovateapp.com>


          New Crowdin updates (directus#9138)

82431ee

* Update source file en-US.yaml

* New translations en-US.yaml (Serbian (Latin))

* New translations en-US.yaml (Portuguese, Brazilian)

* New translations en-US.yaml (Indonesian)

* New translations en-US.yaml (Spanish, Chile)

* New translations en-US.yaml (Thai)

* New translations en-US.yaml (Hindi)

* New translations en-US.yaml (Chinese Traditional)

* New translations en-US.yaml (Spanish, Latin America)

* New translations en-US.yaml (Russian)

* New translations en-US.yaml (Polish)

* New translations en-US.yaml (Portuguese)

* New translations en-US.yaml (Swedish)

* New translations en-US.yaml (Turkish)

* New translations en-US.yaml (Estonian)

* New translations en-US.yaml (Vietnamese)

* New translations en-US.yaml (Chinese Simplified)

* New translations en-US.yaml (French)

* New translations en-US.yaml (Finnish)

* New translations en-US.yaml (Spanish)

* New translations en-US.yaml (Arabic)

* New translations en-US.yaml (Bulgarian)

* New translations en-US.yaml (Catalan)

* New translations en-US.yaml (Danish)

* New translations en-US.yaml (German)

* New translations en-US.yaml (Hungarian)

* New translations en-US.yaml (Italian)

* New translations en-US.yaml (Japanese)

* New translations en-US.yaml (Lithuanian)

* New translations en-US.yaml (Dutch)

* New translations en-US.yaml (Norwegian)

* New translations en-US.yaml (Slovenian)


          Update dependency @types/busboy to v0.3.1 (directus#9096)

2df0c87

Co-authored-by: Renovate Bot <bot@renovateapp.com>


          Update dependency @types/js-yaml to v4.0.4 (directus#9090)

f1de069

Co-authored-by: Renovate Bot <bot@renovateapp.com>


          Update dependency sass to v1.43.3 (directus#9035)

7e00971

Co-authored-by: Renovate Bot <bot@renovateapp.com>


          Docs structure (directus#9071)

3074c69

* WIP

* updates

* docs updates

* structure

* big structure update

* docs module icon change

* in-app docs nav

* more content and structure changes

* Remove redundant

* Fix docs build in app

Co-authored-by: rijkvanzanten <rijkvanzanten@me.com>


          Update dependency stylelint-config-standard to v23 (directus#9026)

18866eb

Co-authored-by: Renovate Bot <bot@renovateapp.com>


          Update dependency stylelint to v14 (directus#9016)

a3f0bac

* Update dependency stylelint to v14

* Update dependency stylelint-scss to v4

* Update dependency stylelint-order to v5

* Undo command change

* Update stylelint command

* Use modern color syntax

Who knew this was already supported everywhere? Awesome!!

* Update stylelint-config-standard

Co-authored-by: Renovate Bot <bot@renovateapp.com>
Co-authored-by: rijkvanzanten <rijkvanzanten@me.com>


          Pin dependency stylelint-config-standard to 23.0.0 (directus#9140)

826e3c2

Co-authored-by: Renovate Bot <bot@renovateapp.com>


          Fix type issues in busboy update

56211a3


          Change v-checkbox background color when disabled (directus#9070)

d8e6c8f

* Change v-checkbox background color when disabled

* Only apply style for block input.


          Don't enforce rgb modern syntax

36e287f


          Use textarea input for textarea placeholder (directus#9041)

db361e8


          Reworked Sass to automatically generate color variations from palette (…

ba6ffd4

…directus#8890)

* Reworked Sass to automatically generate color variations from palette

* Fixed naming scheme for mixing colors

* Updated light theme to use pre-defined steps

* Updated dark theme to use pre-defined steps, added steps at 90 & 110

* Hardcoded -alt color variations, extracted color variation generator to reusable mixin.

Co-authored-by: rijkvanzanten <rijkvanzanten@me.com>


          Update dependency ts-node to v10.4.0 (directus#9083)

e6d65d5

Co-authored-by: Renovate Bot <bot@renovateapp.com>


          Update dependency eslint to v8.1.0 (directus#9091)

b977da6

Co-authored-by: Renovate Bot <bot@renovateapp.com>

Armen Danielyan added 4 commits

November 10, 2021 16:21


          slider rounded edges

bb1f066


          slider rounded edges

67483b7


          slider rounded edges

ed6489e


          slider rounded edges

Contributor Author

adanielyan commented Nov 10, 2021

The slider component is broken in Safari. The reason is that the appearance: none; is overridden by Safari's -webkit-appearance: slider-horizontal;. Adding -webkit-appearance: none; in slider's css fixes the problem. However on every commit it is converted to appearance: none; which doesn't do the trick. Any ideas how to fix that?

Sponsor Member

benhaynes commented Nov 11, 2021

Not sure about the slider in Safari... I'll leave that to the engineers. But the last (small) request, is to decrease the space between the slider and the color chip options. Looking great, thank you for all this awesome work!

Member

rijkvanzanten commented Nov 11, 2021

I think the small border radius is due to the fact that the slider track is made to be 4px high, at which size the border radius is 50%, however, with this thicker track that border radius no longer fits 🙂

The reason is that the appearance: none; is overridden by Safari's -webkit-appearance: slider-horizontal;. Adding -webkit-appearance: none; in slider's css fixes the problem. However on every commit it is converted to appearance: none; which doesn't do the trick. Any ideas how to fix that?

Good one.. I believe that's stylelint that's slightly too aggressive. Here's to hoping the Safari team hurries up with the unprefixed support; it's already support in Technology Preview after all! 😄

Contributor Author

adanielyan commented Nov 12, 2021

Here you go, plus rounded edges


          padding for slider

Contributor Author

adanielyan commented Nov 12, 2021

I think the small border radius is due to the fact that the slider track is made to be 4px high, at which size the border radius is 50%, however, with this thicker track that border radius no longer fits 🙂

In fact the rounded corners were added by the browser (Edge). But I made them fully round anyway. I had to add that to v-slider component, but it will not affect the regular slider if --input-bg-image css var is not explicitly provided in the slider props.

Good one.. I believe that's stylelint that's slightly too aggressive. Here's to hoping the Safari team hurries up with the unprefixed support; it's already support in Technology Preview after all! 😄

If it's good for you it's good for me. I don't use Safari anyway! 😄

azrikahar mentioned this pull request

Add fill & curve options to Insights time series chart #9741

Merged


          Merge branch 'main' of github.com:directus/directus into color-with-a…

2306fbf

…lpha

Contributor Author

adanielyan commented Nov 19, 2021

Hey @benhaynes, is there anything else blocking this PR? If not, maybe it can be merged? Thanks!

Sponsor Member

benhaynes commented Nov 19, 2021

Just the time it takes to have engineering formally review and merge. We've been abnormally busy on cloud the past few weeks... but should have more time starting next week. 👍

Contributor Author

adanielyan commented Nov 19, 2021

Sure, understood. Just wanted to make sure there's nothing else to add/fix.


          renamed Allow Alpha to Opacity

49df9f2

rijkvanzanten requested changes

View reviewed changes

Member

rijkvanzanten left a comment

Couple of tiny nitpicks / tweaks. Otherwise great addition. Thanks @adanielyan! 😄

app/src/interfaces/select-color/select-color.vue Outdated Show resolved Hide resolved

app/src/interfaces/select-color/select-color.vue Outdated Show resolved Hide resolved

app/src/interfaces/select-color/select-color.vue Outdated Show resolved Hide resolved

app/src/interfaces/select-color/select-color.vue Outdated Show resolved Hide resolved

adanielyan and others added 3 commits

December 3, 2021 14:14


          Update app/src/interfaces/select-color/select-color.vue

02d65e7

Co-authored-by: Rijk van Zanten <rijkvanzanten@me.com>


          Update app/src/interfaces/select-color/select-color.vue

cba0b58

Co-authored-by: Rijk van Zanten <rijkvanzanten@me.com>


          implemented suggestions made by @rijkvanzanten

de9ed5d

Contributor Author

adanielyan commented Dec 3, 2021

Thanks for suggestions! They have been implemented.

adanielyan requested a review from rijkvanzanten

December 6, 2021 19:00

Contributor Author

adanielyan commented Dec 28, 2021

Hi, is there any other action I need to take before this can be merged? Sorry, I am not very familiar with the process.

Sponsor Member

benhaynes commented Dec 28, 2021

No, you're good. We've just been super busy with other releases and the holidays. We'll try to get this reviewed/merged ASAP.

rijkvanzanten added this to the v9-next milestone

Contributor Author

adanielyan commented Dec 29, 2021

Great, thanks @benhaynes!


          Merge branch 'main' into color-with-alpha

680ca4f

rijkvanzanten approved these changes

View reviewed changes

rijkvanzanten merged commit 7ad5e76 into directus:main

github-actions bot locked as resolved and limited conversation to collaborators

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

23 participants