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

Fix color picker size #808

Merged
merged 5 commits into from
May 27, 2024
Merged

Fix color picker size #808

merged 5 commits into from
May 27, 2024

Conversation

ArnyminerZ
Copy link
Member

The PR should be in Draft state during development. As soon as it's finished, it should be marked as Ready for review and a reviewer should be chosen.

See also: Writing A Great Pull Request Description

Purpose

Fix color picker size so it matches the text field.

Before:

Screenshot_20240522_154950

After:

image

Note that the guidelines have been added to demonstrate the result, they don't appear on the app obviously.

Short description

  • Forced size to have aspect ratio 1, and fill max height.
  • Added 8dp of padding to the top, so it matches the field's border and not the label's text above.

Checklist

  • The PR has a proper title, description and label.
  • I have self-reviewed the PR.
  • I have added documentation to complex functions and functions that can be used by other modules.
  • I have added reasonable tests or consciously decided to not add tests.

Closes #806

Signed-off-by: Arnau Mora Gras <[email protected]>
@ArnyminerZ ArnyminerZ self-assigned this May 22, 2024
@ArnyminerZ ArnyminerZ requested a review from sunkup May 22, 2024 14:11
@ArnyminerZ ArnyminerZ marked this pull request as ready for review May 22, 2024 14:11
Copy link
Member

@sunkup sunkup left a comment

Choose a reason for hiding this comment

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

Looks good 👍

Copy link
Member

@rfc2822 rfc2822 left a comment

Choose a reason for hiding this comment

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

Looks good; please also add a border (like the text fields). Especially when the color is similar to the background, users may not know that they can tap here to open the color picker.

Signed-off-by: Arnau Mora Gras <[email protected]>
@ArnyminerZ ArnyminerZ requested a review from rfc2822 May 27, 2024 08:49
@rfc2822
Copy link
Member

rfc2822 commented May 27, 2024

I now have used OutlinedButton to have the same border as for the text fields.

Unfortunately I can't set the size "correctly" because the text field has some reserved space for the label which is now included.

It will stay now like this for the release.

@rfc2822 rfc2822 merged commit 9a31835 into main-ose May 27, 2024
6 checks passed
@rfc2822 rfc2822 deleted the fix-color-picker-size branch May 27, 2024 12:53
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.

[gui] Color picker size wrong or not centered.
3 participants