Skip to content

Conversation

@obdulia-losantos
Copy link
Contributor

@obdulia-losantos obdulia-losantos commented May 3, 2025


name: Color preview
about: Add a color preview on readonly color field
assignees: fabiocaccamo


Describe your changes
Added new admin mixin class, ColorMixin, to handle color fields and display a preview of the color in the change list and in the form along with the value when needed.

  • Editable mode
    list-edit
    form-edit
  • Read-only mode
    list-ro
    form-ro

Note: I added the example app to check that everything works as expected, if that's the case I'll remove it before you merge it. (@fabiocaccamo)

Related issue
#91
#104

Checklist before requesting a review

  • I have performed a self-review of my code.
  • I have added tests for the proposed changes.
  • I have run the tests and there are not errors.

@codecov
Copy link

codecov bot commented May 3, 2025

Codecov Report

Attention: Patch coverage is 93.75000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 95.75%. Comparing base (87f5ec6) to head (494c72c).

Files with missing lines Patch % Lines
colorfield/admin.py 96.66% 1 Missing ⚠️
colorfield/widgets.py 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #228      +/-   ##
==========================================
+ Coverage   94.50%   95.75%   +1.24%     
==========================================
  Files           8        9       +1     
  Lines         182      212      +30     
==========================================
+ Hits          172      203      +31     
+ Misses         10        9       -1     
Flag Coverage Δ
unittests 95.75% <93.75%> (+1.24%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@obdulia-losantos obdulia-losantos marked this pull request as draft May 3, 2025 16:20
@obdulia-losantos obdulia-losantos force-pushed the feat-html-ro branch 3 times, most recently from d156fa7 to ac83e92 Compare May 8, 2025 08:34
@obdulia-losantos obdulia-losantos marked this pull request as ready for review May 8, 2025 08:35
@obdulia-losantos
Copy link
Contributor Author

@fabiocaccamo Any feedback?

@fabiocaccamo
Copy link
Owner

@obdulia-losantos sorry for the late feedback, I had not the time to try it yet, a quick feedback:

  • I would make the read-only color preview a square of the exact same size of the color square on the right of the input field.

  • When the readonly color has alpha, is the alpha background pattern visible?

@obdulia-losantos
Copy link
Contributor Author

@fabiocaccamo

  1. I made the read-only much more similar to the input one but I moved the square to the left side
  2. The alpha background pattern is visible in the input but not in the read-only field

image
image

@fabiocaccamo fabiocaccamo self-assigned this Jun 11, 2025
@fabiocaccamo fabiocaccamo self-requested a review June 11, 2025 21:20
@fabiocaccamo fabiocaccamo added the enhancement New feature or request label Jun 11, 2025
@fabiocaccamo fabiocaccamo moved this to In Progress in Open Source Jun 11, 2025
@fabiocaccamo
Copy link
Owner

@obdulia-losantos I finally tested this PR and also pushed some changes.

I would like to ask you some questions:


  1. Why in the image below, the second field doesn't display anything?
Screenshot 2025-06-12 at 00 23 45
  1. In the admin mixin, you overwritten 3 templates, why? Is there something I'm missing? I found an alternative solution because I don't want to overwrite admin templates to avoid potential conflicts, you can find some TODO comments in ColorAdminMixin.

@obdulia-losantos
Copy link
Contributor Author

@fabiocaccamo I fixed the tests

@obdulia-losantos
Copy link
Contributor Author

@fabiocaccamo any comment?

@obdulia-losantos
Copy link
Contributor Author

@fabiocaccamo 🤔

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

Labels

enhancement New feature or request

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

2 participants