Add UI enhancements: icons, favicon, profile page, clear filters#28
Add UI enhancements: icons, favicon, profile page, clear filters#28
Conversation
CSS and webfont files downloaded from cdnjs, served locally via static-global. Paths use ../fonts/ relative references.
SVG open-book icon with purple background, PNG variants at multiple sizes, and ICO. Added FA CSS + favicon link tags to base.html head.
- Favicon SVG as logo next to site name - FA icons on all nav links (upload, book scans, opinions) - User dropdown with profile link and sign out - Click-outside handler to close dropdown
- Add gap-1.5 to .btn base class for icon-text spacing - Add hover:text-white to .btn-primary for <a> tag usage - Add icons to Cancel, Upload, Save, Submit Review, Edit in admin buttons
- Add filter/clear buttons on a separate row below filter inputs - Add FA icons on Upload, Filter, and Clear buttons - Clear filters button always visible
- ProfileForm (first_name, last_name, email) - profile and password_change views with templates - URL routes at /profile/ and /profile/password/
Remove is_staff gate from scan_detail and opinion_detail views so all logged-in users can submit reviews.
- TestProfile: login required, GET renders, POST updates fields - TestPasswordChange: login required, GET renders, valid/invalid POST - Update review form tests to expect form for non-staff users
for more information, see https://pre-commit.ci
grossir
left a comment
grossir
left a comment
There was a problem hiding this comment.
one important comment on who can POST a ScanReview
| opinion_count = scan.opinions.count() | ||
|
|
||
| review_form = None | ||
| if request.user.is_staff and scan.status != Status.APPROVED: |
There was a problem hiding this comment.
dropping the is_staff check means any logged in user can perform ("POST") a scan review.
I think you should move that condition here:
if request.method == "POST" and request_user.is_staff:
There was a problem hiding this comment.
In this case, I removed is_staff because I believe the idea is for them to do everything from the frontend without needing to add permissions or having to go directly to the admin. Now, if we want to use the is_staff flag, give admin access, and have more granular control over actions, we could use is_staff but also create groups with a set of permissions.
For the moment i can remove the profile changes to get this up and running.
What do you think about this?
There was a problem hiding this comment.
I agree they should do everything on the frontend
The problem is the current code is allowing anyone that can log in, includding the scanners, to review a scan. Meaning, they could approve their own scans?
That's why I was suggestion to add this if request.method == "POST" and request_user.is_staff. So, only is_staff users can "POST" a review
if it is a "GET", to look at a review, anyone can do it.
There was a problem hiding this comment.
That is a good question, I would hope that the person who did the scan would be responsible for reviewing it, but I think it is better to ask, for now we can leave it as you mentioned so that only those who have the is_staff can do it, initially they would not have that flag so they cannot see the admin.
- Add @fortawesome/fontawesome-free as npm devDependency - Add build:fa scripts to copy CSS and webfonts from node_modules - Update all icon classes from `fa` to `fa-solid` prefix - Rename deprecated icons (file-text, cog, sign-out, times, search) - Delete vendored FA 4 CSS and font files - Gitignore generated FA assets (same pattern as Tailwind)
2 participants
Summary
Paired with @quevon24 and Claude Code