Report list photos, image lightbox, announcement label (#118) - #22
Conversation
theobong
changed the title
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| .lightbox-overlay { background: rgba(26, 23, 20, 0.78); padding: 32px; } | ||
| .lightbox { | ||
| position: relative; | ||
| width: fit-content; max-width: min(1100px, 100%); | ||
| min-width: min(320px, 100%); min-height: min(240px, 100%); | ||
| display: flex; align-items: center; justify-content: center; | ||
| animation: modal-pop 200ms cubic-bezier(0.25, 0.8, 0.3, 1); | ||
| } | ||
| .lightbox-face { | ||
| display: flex; flex-direction: column; align-items: center; justify-content: center; | ||
| gap: 8px; | ||
| width: min(320px, 100%); | ||
| padding: 24px; | ||
| text-align: center; | ||
| color: #fff; | ||
| font-size: 12.5px; | ||
| background: rgba(26, 23, 20, 0.55); | ||
| border-radius: 12px; | ||
| } | ||
| .lightbox-face-title { font-weight: 700; font-size: 14px; } | ||
| .lightbox-face-sub { color: rgba(255, 255, 255, 0.72); line-height: 1.5; } | ||
| .lightbox-face-btn { | ||
| margin-top: 4px; | ||
| padding: 7px 14px; border-radius: 10px; | ||
| font-size: 12px; font-weight: 700; | ||
| color: #fff; background: rgba(26, 23, 20, 0.72); | ||
| transition: background-color 120ms ease; | ||
| } | ||
| .lightbox-face-btn:hover { background: rgba(26, 23, 20, 0.9); } | ||
| .lightbox-img { | ||
| max-width: 100%; max-height: calc(100vh - 64px); | ||
| width: auto; height: auto; display: block; | ||
| border-radius: 12px; | ||
| background: var(--paper-2); | ||
| box-shadow: 0 24px 64px rgba(26, 23, 20, 0.45); | ||
| } | ||
| .lightbox-img.pending { display: none; } | ||
| .lightbox-bar { | ||
| position: absolute; top: 10px; right: 10px; z-index: 1; | ||
| display: flex; align-items: center; gap: 8px; | ||
| } | ||
| .lightbox-count { | ||
| font-size: 10.5px; font-weight: 700; color: #fff; | ||
| background: rgba(26, 23, 20, 0.72); | ||
| padding: 4px 8px; border-radius: 8px; | ||
| } | ||
| .lightbox-btn { | ||
| width: 34px; height: 34px; border-radius: 10px; flex-shrink: 0; | ||
| display: flex; align-items: center; justify-content: center; | ||
| color: #fff; background: rgba(26, 23, 20, 0.72); | ||
| transition: background-color 120ms ease; | ||
| } | ||
| .lightbox-btn:hover { background: rgba(26, 23, 20, 0.9); } | ||
| .lightbox-prev, .lightbox-next { position: absolute; top: 50%; transform: translateY(-50%); } | ||
| .lightbox-prev { left: 10px; } | ||
| .lightbox-next { right: 10px; } |
There was a problem hiding this comment.
The lightbox introduces raw dimensions and literal colors, including 32px, #fff, and several rgba(...) values, rather than the existing CSS variables. This violates the frontend styling directive. Replace these values with the appropriate spacing, sizing, color, radius, and typography tokens; this repository requirement must be satisfied before merging.
Rule Used: # civfix review rules civfix is a live civic-tech platform that will hold government contracts. Review every PR for correctness, security and performance. Flag real defects with evidence; skip style nits that lint already covers. ## Repos - **... (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/admin/src/styles/admin.css
Line: 2948-3003
Comment:
**Use design tokens**
The lightbox introduces raw dimensions and literal colors, including `32px`, `#fff`, and several `rgba(...)` values, rather than the existing CSS variables. This violates the frontend styling directive. Replace these values with the appropriate spacing, sizing, color, radius, and typography tokens; this repository requirement must be satisfied before merging.
**Rule Used:** # civfix review rules civfix is a live civic-tech platform that will hold government contracts. Review every PR for **correctness, security and performance**. Flag real defects with evidence; skip style nits that lint already covers. ## Repos - **... ([source](https://app.greptile.com/civfix/-/custom-context?memory=39a53925-3d93-4c82-980e-27b67393717d))
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| </button> | ||
| </div> | ||
| {status !== "failed" && ( | ||
| // eslint-disable-next-line @next/next/no-img-element |
There was a problem hiding this comment.
The new lightbox adds an inline lint-suppression comment, and the new report chat image rendering adds the same kind of comment. This violates the directive that new code contain no comments. Use an implementation or project configuration that does not require new inline comments; this repository requirement must be satisfied before merging.
Rule Used: # civfix review rules civfix is a live civic-tech platform that will hold government contracts. Review every PR for correctness, security and performance. Flag real defects with evidence; skip style nits that lint already covers. ## Repos - **... (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/admin/src/components/shared/lightbox.tsx
Line: 128
Comment:
**Remove new code comments**
The new lightbox adds an inline lint-suppression comment, and the new report chat image rendering adds the same kind of comment. This violates the directive that new code contain no comments. Use an implementation or project configuration that does not require new inline comments; this repository requirement must be satisfied before merging.
**Rule Used:** # civfix review rules civfix is a live civic-tech platform that will hold government contracts. Review every PR for **correctness, security and performance**. Flag real defects with evidence; skip style nits that lint already covers. ## Repos - **... ([source](https://app.greptile.com/civfix/-/custom-context?memory=39a53925-3d93-4c82-980e-27b67393717d))
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| @@ -0,0 +1,177 @@ | |||
| "use client" | |||
There was a problem hiding this comment.
This change exceeds the repository's 400-line pull-request limit, but its description does not include the required Size: justification. Add the explicit justification or split the modal accessibility, report-photo viewer, and announcement-label work into smaller changes; this repository requirement must be satisfied before merging.
Rule Used: # civfix review rules civfix is a live civic-tech platform that will hold government contracts. Review every PR for correctness, security and performance. Flag real defects with evidence; skip style nits that lint already covers. ## Repos - **... (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/admin/src/components/shared/lightbox.tsx
Line: 1
Comment:
**Justify the PR size**
This change exceeds the repository's 400-line pull-request limit, but its description does not include the required `Size:` justification. Add the explicit justification or split the modal accessibility, report-photo viewer, and announcement-label work into smaller changes; this repository requirement must be satisfied before merging.
**Rule Used:** # civfix review rules civfix is a live civic-tech platform that will hold government contracts. Review every PR for **correctness, security and performance**. Flag real defects with evidence; skip style nits that lint already covers. ## Repos - **... ([source](https://app.greptile.com/civfix/-/custom-context?memory=39a53925-3d93-4c82-980e-27b67393717d))
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
2 participants
Resolves civfix/issue-tracker#118
What changed
In the operator dashboard, report rows in the Reports list now lead with the report's own photo (falling back to the category pin when there is no photo), and every report image — the preview photo, the "Photos" gallery, and photo attachments in the report chat — can be clicked to expand in a full-screen viewer with keyboard support. The dashboard also now names the new "Announcement" kind of host message in the broadcast log. It additionally moves onto the newer shared contract the other two PRs ship, which changes nothing an operator can see. Admin only.
Before you start
dev/run.sh admin+dev/run.sh admin-login)Verify
Report photos and the image viewer — [Admin]
Announcements in the broadcast log — [Admin]
Regression
Escape behavior on admin pages — [Admin]
Confirm and prompt dialogs across the dashboard — [Admin]
Reports list paging, filters and search — [Admin]
Report detail actions next to the images — [Admin]
Video and non-image media — [Admin]
Host messaging page around the log — [Admin]
Jurisdiction rows and Moderation media — [Admin]
Older backend tolerance — [Admin]
Not covered