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

fix(NcColorPicker): Make the color circle size depend on clickable area #5816

Merged
merged 1 commit into from
Jul 22, 2024
Merged

fix(NcColorPicker): Make the color circle size depend on clickable area #5816

merged 1 commit into from
Jul 22, 2024

Conversation

ChristophWurst
Copy link
Contributor

@ChristophWurst ChristophWurst commented Jul 17, 2024 •
edited
Loading

☑️ Resolves

The size of the circles is hard coded but the grid size depends on the clickable area. So I made the size dependent as well. I've tested this with a clickable area of 34. It works with the old 44 as well:

image

🖼️ Screenshots

🏚️ Before 🏡 After
Bildschirmfoto vom 2024-07-17 13-53-00 Bildschirmfoto vom 2024-07-17 13-52-37

🏁 Checklist

  • ⛑️ Tests are included or are not applicable
  • 📘 Component documentation has been extended, updated or is not applicable
  • 3️⃣ Backport to next requested with a Vue 3 upgrade

@ChristophWurst ChristophWurst added bug Something isn't working 3. to review Waiting for reviews regression Regression of a previous working feature feature: colorpicker Related to the colorpicker component labels Jul 17, 2024
@ChristophWurst ChristophWurst self-assigned this Jul 17, 2024
@ChristophWurst ChristophWurst force-pushed the fix/colorpicker/clickable-area-agnostic-color-circles branch from 5551f38 to f4b042f Compare July 17, 2024 11:58
@ChristophWurst
Copy link
Contributor Author

/backport to next

@ChristophWurst
Copy link
Contributor Author

This is probably just a leftover of a24ea0a.

st3iny
st3iny approved these changes Jul 17, 2024
View reviewed changes
Copy link
Contributor

@st3iny st3iny left a comment

Choose a reason for hiding this comment

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

Looks much better

@ChristophWurst ChristophWurst added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Jul 17, 2024
susnux
susnux approved these changes Jul 17, 2024
View reviewed changes
Copy link
Contributor

@susnux susnux left a comment

Choose a reason for hiding this comment

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

some future improvments required (there are still magic values like "10px"). but much better!

@ChristophWurst
fix(NcColorPicker): Make the color circle size depend on clickable area
caed40b 
Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
@ChristophWurst ChristophWurst force-pushed the fix/colorpicker/clickable-area-agnostic-color-circles branch from f4b042f to caed40b Compare July 22, 2024 12:53
@ChristophWurst ChristophWurst merged commit 314b5e9 into master Jul 22, 2024
19 checks passed
@ChristophWurst ChristophWurst deleted the fix/colorpicker/clickable-area-agnostic-color-circles branch July 22, 2024 12:55
@backportbot backportbot bot mentioned this pull request Jul 22, 2024
Merged
@Antreesy Antreesy mentioned this pull request Jul 22, 2024
Merged
This was referenced Aug 14, 2024
Open
Open
@Stanislav1975 Stanislav1975 mentioned this pull request Aug 26, 2024
Open
This was referenced Sep 6, 2024
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AndyScherzinger AndyScherzinger AndyScherzinger approved these changes

@st3iny st3iny st3iny approved these changes

@susnux susnux susnux approved these changes

@marcoambrosini marcoambrosini marcoambrosini approved these changes

@Pytal Pytal Awaiting requested review from Pytal

@GVodyanov GVodyanov Awaiting requested review from GVodyanov

Assignees

@ChristophWurst ChristophWurst

Labels
4. to release Ready to be released and/or waiting for tests to finish bug Something isn't working feature: colorpicker Related to the colorpicker component regression Regression of a previous working feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ChristophWurst @AndyScherzinger @st3iny @susnux @marcoambrosini