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

Updated the layout of checkbox #300

Merged
merged 2 commits into from
Jan 12, 2022

Conversation

piyushsinghania
Copy link
Contributor

The checkbox was breaking initially-
image

Update the checkbox slightly

off -
image

checked -
image

cc: @ankitsinghaniyaz @18alantom

Copy link
Member

@18alantom 18alantom left a comment

Choose a reason for hiding this comment

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

@piyushsinghania I didn't know the checkbox broke on linux. Here are a few screenshots of how it appears on my system:

Screenshot 2022-01-12 at 11 26 49

Screenshot 2022-01-12 at 11 27 32

Screenshot 2022-01-12 at 11 27 38

Screenshot 2022-01-12 at 11 27 55

Screenshot 2022-01-12 at 11 28 01

Can you make it look this way without the UX breaking? Primarily:

  • Maintain width and styling with other elements, the report checkbox should have BG and match the width, as of now even I'm not a fan of the background appearance but will change them all together when doing a UI fix later on.
  • Don't use the accent color for the checkbox, it makes the checkbox standout among other elements even when no special importance is given to it. As of now don't change the coloring.

@piyushsinghania
Copy link
Contributor Author

piyushsinghania commented Jan 12, 2022

I have made the suggested changes @18alantom
cc: @ankitsinghaniyaz

image

image

image

@18alantom
Copy link
Member

LGTM, failing build is unrelated, hence merging.

@18alantom 18alantom merged commit 8958d96 into frappe:master Jan 12, 2022
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.

2 participants