Add _ keyboard shortcut to raise whites, add dialog to confirm quit when a batch is defined#86
Conversation
…hen a batch is defined
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 48 minutes and 14 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
WalkthroughThe 1.6.3 release updates auto-adjust keybindings and quit-with-batches safeguards. The highlight step scaling doubles (0.07 → 0.14), a new underscore key raises whites by 14 points, and window close now checks for defined batches, prompting confirmation before exit. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant KeystrokeHandler as Keystroke Handler<br/>(keystrokes.py)
participant Controller as AppController<br/>(app.py)
participant Preview as Live Preview
User->>KeystrokeHandler: Press `_` key
KeystrokeHandler->>Controller: raise_auto_adjust_whites()
Controller->>Controller: Ensure active auto-adjust state
Controller->>Controller: Decrement extra_highlight_steps by 1
Controller->>Preview: Refresh live preview
Preview->>User: Display updated whites adjustment
sequenceDiagram
participant User
participant Window as Main Window<br/>(Main.qml)
participant Controller as AppController<br/>(app.py)
participant Dialog as QuitBatchesDialog<br/>(QuitBatchesDialog.qml)
participant Qt as Qt Runtime
User->>Window: Initiate close (Alt+F4 or menu)
alt allowCloseWithBatches is false
Window->>Controller: get_defined_batch_count()
Controller-->>Window: Returns batch count
alt Batch count > 0
Window->>Dialog: Set batchCount property
Window->>Dialog: Open dialog
Dialog->>User: Prompt confirmation
alt User confirms (Quit Anyway)
User->>Dialog: Click "Quit Anyway"
Dialog->>Window: Emit quitConfirmed()
Window->>Window: Set allowCloseWithBatches = true
Window->>Qt: Qt.quit()
else User cancels
User->>Dialog: Click "Cancel"
Dialog->>User: Close dialog
Window->>Window: Close cancelled
end
end
else allowCloseWithBatches is true
Window->>Qt: Allow window close
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
20-20:⚠️ Potential issue | 🟡 MinorStale step size and missing
_mention in the Features blurb.The Quick Auto Adjust bullet still describes
-as using 7-point steps and omits the new_shortcut, which contradicts the updated Keyboard Shortcuts section (lines 145‑146) and the 1.6.3 changelog entry where-/_both move highlights/whites in 14-point steps.📝 Proposed fix
-- **Quick Auto Adjust:** Press `l` for quick auto-levels, `L` for auto white balance + auto-levels together, `A` for auto white balance, `-` to keep darkening the highlight/white side in 7-point steps, and `=` to deepen the shadow side in 7-point steps. These update the live in-memory edit session immediately and save once when you navigate away, start a drag, or explicitly save. +- **Quick Auto Adjust:** Press `l` for quick auto-levels, `L` for auto white balance + auto-levels together, `A` for auto white balance, `-` to keep darkening the highlight/white side in 14-point steps, `_` to raise whites in 14-point steps, and `=` to deepen the shadow side in 7-point steps. These update the live in-memory edit session immediately and save once when you navigate away, start a drag, or explicitly save.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` at line 20, Update the "Quick Auto Adjust" feature blurb so it matches the Keyboard Shortcuts and changelog: change the description for the '-' key from "7-point steps" to "14-point steps" and mention the new '_' shortcut (e.g., "`-`/`_` to keep darkening the highlight/white side in 14-point steps"), leaving the rest of the shortcuts (`l`, `L`, `A`, `=`) as-is so the bulleted sentence aligns with the Keyboard Shortcuts section and 1.6.3 changelog.
🧹 Nitpick comments (2)
faststack/qml/QuitBatchesDialog.qml (2)
10-11: Fixed pixel width/height may clip under larger system fonts or translations.
width: 450; height: 250is fixed regardless of content. If the batch-count message gets longer (localization, very large counts) or the system DPI/font scale is higher, the action row can get clipped. Consider usingimplicitWidth/implicitHeightdriven by the content column, or at leastMath.min(450, parent.width * 0.9)/implicitHeight: contentItem.implicitHeight.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@faststack/qml/QuitBatchesDialog.qml` around lines 10 - 11, QuitBatchesDialog.qml uses fixed width: 450 and height: 250 which can clip when text is longer or fonts/DPI increase; change the sizing to follow content by replacing fixed width/height with responsive sizing — e.g., set implicitWidth or width to something derived from the content column (contentItem.implicitWidth or Math.min(450, parent.width * 0.9)) and set implicitHeight to contentItem.implicitHeight (or use Math.min as needed) so the dialog (QuitBatchesDialog) and its action row/Column content expand to fit localized/large text.
19-24: Theme-agnostic hardcoded colors.
backgroundColor/textColorare themed via bindings fromMain.qml, but the dialog frame border (#404040), Cancel button (#444444/#666666/#555555), and the "Quit Anyway" button reds are hardcoded. In light theme this will look inconsistent with the rest of the app (compare withrecycleBinCleanupDialog, which keys colors offroot.isDarkTheme). Consider exposing adarkThemeboolean property (or dedicated border/button color properties) and wiring it fromMain.qmlto keep the dialog theme-consistent.Also applies to: 54-57, 73-76
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@faststack/qml/QuitBatchesDialog.qml` around lines 19 - 24, The dialog uses hardcoded colors for the frame border (border.color "#404040" in the background Rectangle) and for the Cancel and "Quit Anyway" buttons (lines noted around 54-57 and 73-76), so add theme-aware properties (e.g., property bool darkTheme or dedicated color properties like property color frameBorderColor, cancelBgColor, cancelHoverColor, cancelPressedColor, quitBgColor) to QuitBatchesDialog.qml and replace the hardcoded hex values with bindings to those properties; then wire those properties from Main.qml (or whichever parent exposes root.isDarkTheme) so the dialog picks dark/light colors from the app theme rather than using fixed hex codes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@faststack/app.py`:
- Around line 5147-5160: The current get_defined_batch_count() uses
self.image_files emptiness and clamps ranges incorrectly, causing out-of-range
batches like [20,30] to become a fake hit; change the early guard to check
self.batches (e.g., if not self.batches: return 0) instead of if not
self.image_files, then compute max_index = len(self.image_files) - 1 and for
each (start,end) in self.batches intersect properly using clamped_start =
max(start, 0) and clamped_end = min(end, max_index), only adding when
clamped_start <= clamped_end so disjoint/stale ranges do not produce spurious
counts.
---
Outside diff comments:
In `@README.md`:
- Line 20: Update the "Quick Auto Adjust" feature blurb so it matches the
Keyboard Shortcuts and changelog: change the description for the '-' key from
"7-point steps" to "14-point steps" and mention the new '_' shortcut (e.g.,
"`-`/`_` to keep darkening the highlight/white side in 14-point steps"), leaving
the rest of the shortcuts (`l`, `L`, `A`, `=`) as-is so the bulleted sentence
aligns with the Keyboard Shortcuts section and 1.6.3 changelog.
---
Nitpick comments:
In `@faststack/qml/QuitBatchesDialog.qml`:
- Around line 10-11: QuitBatchesDialog.qml uses fixed width: 450 and height: 250
which can clip when text is longer or fonts/DPI increase; change the sizing to
follow content by replacing fixed width/height with responsive sizing — e.g.,
set implicitWidth or width to something derived from the content column
(contentItem.implicitWidth or Math.min(450, parent.width * 0.9)) and set
implicitHeight to contentItem.implicitHeight (or use Math.min as needed) so the
dialog (QuitBatchesDialog) and its action row/Column content expand to fit
localized/large text.
- Around line 19-24: The dialog uses hardcoded colors for the frame border
(border.color "#404040" in the background Rectangle) and for the Cancel and
"Quit Anyway" buttons (lines noted around 54-57 and 73-76), so add theme-aware
properties (e.g., property bool darkTheme or dedicated color properties like
property color frameBorderColor, cancelBgColor, cancelHoverColor,
cancelPressedColor, quitBgColor) to QuitBatchesDialog.qml and replace the
hardcoded hex values with bindings to those properties; then wire those
properties from Main.qml (or whichever parent exposes root.isDarkTheme) so the
dialog picks dark/light colors from the app theme rather than using fixed hex
codes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c24105a4-3649-4c44-828f-3e2f43428300
📒 Files selected for processing (6)
ChangeLog.mdREADME.mdfaststack/app.pyfaststack/qml/Main.qmlfaststack/qml/QuitBatchesDialog.qmlfaststack/ui/keystrokes.py
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 20a66849ae
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| clamped_start = max(0, min(start, max_index)) | ||
| clamped_end = max(0, min(end, max_index)) | ||
| if clamped_start <= clamped_end: |
There was a problem hiding this comment.
The new get_defined_batch_count() logic clamps both ends independently, which converts fully out-of-range batches into a fake 1-image range (for example [120,130] with max_index=99 becomes [99,99]). When batch indices become stale after image-list shrink/reindex events, this overcounts and can trigger a false quit-warning dialog even though no valid batched images remain. Count only the actual intersection with [0, max_index] (i.e., start=max(start,0), end=min(end,max_index)) so non-overlapping ranges contribute zero.
Useful? React with 👍 / 👎.
1 participant
Summary by CodeRabbit
Release Notes
New Features
Documentation