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 checkbox in dark mode #2051

Closed

Conversation

sotten
Copy link
Contributor

@sotten sotten commented Nov 11, 2024

All Submissions:

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?

The checkboxes do not work properly in dark mode with Tailwind. Resolves #1199

Style testet with Chromium Version 129 and [email protected]:

Dark Light
Unchecked Bildschirmfoto vom 2024-11-11 22-38-33 Bildschirmfoto vom 2024-11-11 22-39-30
Checked Bildschirmfoto vom 2024-11-11 22-36-31 Bildschirmfoto vom 2024-11-11 22-39-10
Unchecked focus Bildschirmfoto vom 2024-11-11 22-38-51 Bildschirmfoto vom 2024-11-11 22-39-43
Checked focus Bildschirmfoto vom 2024-11-11 22-37-53 Bildschirmfoto vom 2024-11-11 22-39-24

Copy link

codecov bot commented Nov 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.86%. Comparing base (0c8ec45) to head (304bb0a).
Report is 202 commits behind head on development.

Additional details and impacted files
@@                Coverage Diff                @@
##             development    #2051      +/-   ##
=================================================
+ Coverage          87.30%   87.86%   +0.56%     
- Complexity          1672     1934     +262     
=================================================
  Files                150      184      +34     
  Lines               3891     4426     +535     
=================================================
+ Hits                3397     3889     +492     
- Misses               494      537      +43     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

@sotten sotten force-pushed the fix-checkboxes-in-dark-mode branch from 082beea to 304bb0a Compare November 12, 2024 19:59
@lrljoe
Copy link
Collaborator

lrljoe commented Nov 17, 2024

Will review tomorrow, as with the shifting of the order of the classes, I just need to double check that any existing classes haven't been replaced.

@sotten
Copy link
Contributor Author

sotten commented Nov 19, 2024

I have used the order of classes recommended by Tailwind:
https://tailwindcss.com/blog/automatic-class-sorting-with-prettier#how-classes-are-sorted

I use the following plugin from Tailwindlabs local: https://github.com/tailwindlabs/prettier-plugin-tailwindcss

I can change the order back to the original if you like.
Maybe it would be useful for the future to include the Prettier plugin in github action. Helpful tutorial: https://akhilaariyachandra.com/blog/prettier-in-github-actions

@lrljoe
Copy link
Collaborator

lrljoe commented Nov 20, 2024

Some of the below are superfluous, but any removals will need to be validated.

resources/views/components/table/td/bulk-actions.blade.php

Removed From Current
dark:text-white dark:hover:bg-gray-600 dark:focus:bg-gray-600

Added To Current
dark:checked:bg-indigo-500

resources/views/components/table/th/bulk-actions.blade.php

Removed From Current
dark:text-white dark:hover:bg-gray-600 dark:focus:bg-gray-600

Added To Current
dark:checked:bg-indigo-500

resources/views/components/tools/filters/multi-select.blade.php

Removed From Current
dark:text-white dark:hover:bg-gray-600 dark:focus:bg-gray-600

Added To Current
dark:checked:bg-indigo-500

Removed From Current
dark:text-white dark:hover:bg-gray-600 dark:focus:bg-gray-600
Added To Current
dark:checked:bg-indigo-500

resources/views/components/tools/toolbar/items/column-select.blade.php

Removed From Current
class=" dark:text-white dark:hover:bg-gray-600 dark:focus:bg-gray-600 "

Added To Current
class=" dark:checked:bg-indigo-500 "

Removed From Current
class=" dark:text-white dark:hover:bg-gray-600 dark:focus:bg-gray-600 "

Added To Current
class=" dark:checked:bg-indigo-500 "

@sotten
Copy link
Contributor Author

sotten commented Nov 20, 2024

Yes, that is intentional.

@lrljoe
Copy link
Collaborator

lrljoe commented Nov 20, 2024

Yes, that is intentional.

Not so much objection, more to remind myself when I review this further

@lrljoe
Copy link
Collaborator

lrljoe commented Nov 22, 2024

Having reviewed this, it removes an existing behaviour (hover/focus classes on checkboxes), which is what the ~2m projects using the package have come to expect.

Therefore this PR is rejected.

There is an upcoming update that will add more customisation in for users who wish to change the default attributes on filter inputs.

@lrljoe lrljoe closed this Nov 22, 2024
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