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.

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

Add advanced fields to color picker #3324

Merged
merged 1 commit into from
Oct 4, 2022
Merged

Add advanced fields to color picker #3324

merged 1 commit into from
Oct 4, 2022

Conversation

Pytal
Copy link
Contributor

@Pytal Pytal commented Oct 1, 2022

Before After
image image
RGB and HSL

RGB
image

HSL
image

@Pytal
Add advanced fields to color picker
61ddd09 
Signed-off-by: Christopher Ng <chrng8@gmail.com>
@Pytal Pytal added enhancement New feature or request 3. to review Waiting for reviews feature: colorpicker Related to the colorpicker component labels Oct 1, 2022
@Pytal Pytal added this to the 7.0.0 milestone Oct 1, 2022
@Pytal Pytal requested review from skjnldsv, marcoambrosini and a team October 1, 2022 03:09
@Pytal Pytal self-assigned this Oct 1, 2022
@Pytal Pytal requested review from PVince81 and artonge and removed request for a team October 1, 2022 03:09
szaimen
szaimen approved these changes Oct 1, 2022
View reviewed changes
Copy link
Contributor

@szaimen szaimen left a comment

Choose a reason for hiding this comment

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

width looks strange but otherwise good:
image

@Pytal
Copy link
Contributor Author

Pytal commented Oct 4, 2022

Cypress failures unrelated, see #3325

@Pytal Pytal added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Oct 4, 2022
@Pytal Pytal merged commit c4271b1 into master Oct 4, 2022
@Pytal Pytal deleted the enh/color-picker-advanced-fields branch October 4, 2022 04:09
@Pytal Pytal mentioned this pull request Oct 6, 2022
Merged
6 tasks
@st3iny st3iny mentioned this pull request Feb 23, 2023
Closed
@ChristophWurst ChristophWurst added the breaking PR that requires a new major version label Feb 23, 2023
@ChristophWurst
Copy link
Contributor

FYI as seen in nextcloud/calendar#5013 this change is not backwards compatible for apps that used the old picker

@raimund-schluessler
Copy link
Contributor

FYI as seen in nextcloud/calendar#5013 this change is not backwards compatible for apps that used the old picker

I am a bit puzzled. Before this change it wasn't possible to show the advanced fields at all. :disable-fields="true" did not offer to show them. Are you sure they were visible?

@Pytal
Copy link
Contributor Author

Pytal commented Feb 23, 2023

I am a bit puzzled. Before this change it wasn't possible to show the advanced fields at all. :disable-fields="true" did not offer to show them. Are you sure they were visible?

Precisely, the fields were never visible before and this added them

Seems that the native color picker was used before nextcloud/calendar@e7c4897#diff-cb41e24846d5e15e56c4f885bb5c83fcf872d22c308ef7a529409334bef6c891L81-L88 @ChristophWurst

@Pytal Pytal removed the breaking PR that requires a new major version label Feb 23, 2023
@ChristophWurst
Copy link
Contributor

My bad

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

@marcoambrosini marcoambrosini marcoambrosini approved these changes

@szaimen szaimen szaimen approved these changes

@skjnldsv skjnldsv Awaiting requested review from skjnldsv

@PVince81 PVince81 Awaiting requested review from PVince81

@artonge artonge Awaiting requested review from artonge

Assignees

@Pytal Pytal

Labels
4. to release Ready to be released and/or waiting for tests to finish enhancement New feature or request feature: colorpicker Related to the colorpicker component
Projects
None yet
Milestone
7.0.0
Development

Successfully merging this pull request may close these issues.
5 participants
@Pytal @ChristophWurst @raimund-schluessler @marcoambrosini @szaimen