🎨 Palette: Add empty states to layout draft and visible window lists#13
🎨 Palette: Add empty states to layout draft and visible window lists#13inherent-vice wants to merge 1 commit into
Conversation
Co-authored-by: agno7766 <125467265+agno7766@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
📝 WalkthroughWalkthroughThis PR implements empty-state UI patterns for two Treeview widgets in a Window Manager application. A UX convention is documented, then applied to available and draft window lists via placeholder rows that are displayed when lists are empty or match no filters. ChangesEmpty-state UI for window lists
🎯 2 (Simple) | ⏱️ ~12 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![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: 4b830995d0
ℹ️ 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".
| tree.insert( | ||
| "", | ||
| "end", | ||
| iid="__empty__", |
There was a problem hiding this comment.
When the layout draft is empty, this placeholder row becomes the row returned by identify_row; right-clicking it calls _show_draft_context_menu() -> _select_draft_window_from_event(), which still does int(row_id) and raises ValueError for "__empty__". This breaks the context menu precisely in the new empty-state scenario, so the placeholder should be ignored in the selection helper as well as in the checkbox toggle handler.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Window_Modern.py`:
- Around line 5633-5647: The placeholder row is inserted with iid="__empty__"
which breaks _select_draft_window_from_event() because that function assumes
numeric row ids and calls int(row_id); modify the selection handler to guard
against the placeholder by checking for the "__empty__" iid (or non-digit ids)
before converting to int and return early (no selection) when detected, or
alternatively ensure the placeholder is inserted with a numeric-but-invalid iid
and handled accordingly; update references in _select_draft_window_from_event()
and where draft_windows are iterated to skip or ignore the "__empty__" row.
- Around line 5540-5549: render_available_windows() inserts a placeholder row
with iid="__empty__", but other interaction paths still act on that row; update
_add_available_window_from_event() and _show_available_context_menu() (and any
handler that processes tree selections for the available-tree) to check the
selected item's iid and early-return when iid == "__empty__" so placeholder rows
are ignored consistently and no actions/status updates are performed for the
placeholder.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 36de49ff-b4f2-40aa-9cfc-e3382cc672f9
📒 Files selected for processing (2)
.Jules/palette.mdWindow_Modern.py
| if not has_items: | ||
| values = ("", "No visible windows found") | ||
| if "size" in columns: | ||
| values = ("", "No visible windows found", "") | ||
| tree.insert( | ||
| "", | ||
| "end", | ||
| iid="__empty__", | ||
| values=values, | ||
| ) |
There was a problem hiding this comment.
Guard __empty__ in all available-tree interaction paths, not just checkbox toggle.
render_available_windows() adds iid="__empty__", but _add_available_window_from_event() and _show_available_context_menu() can still select it. That violates the “ignore placeholder interactions” contract and yields misleading actions/status.
Suggested fix
def _add_available_window_from_event(self, event) -> None:
if self.available_tree is None:
return
row_id = self.available_tree.identify_row(event.y)
- if not row_id:
+ if not row_id or row_id == "__empty__":
return
self.selected_available_hwnds = {row_id}
self.add_selected_windows_to_draft()
def _show_available_context_menu(self, event: tk.Event) -> str:
if self.available_tree is None:
return "break"
row_id = self.available_tree.identify_row(event.y)
- if row_id:
+ if row_id and row_id != "__empty__":
self.selected_available_hwnds = {row_id}
self.available_tree.selection_set(row_id)
self.available_tree.focus(row_id)
self.render_available_windows()
+ elif row_id == "__empty__":
+ self.selected_available_hwnds.clear()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Window_Modern.py` around lines 5540 - 5549, render_available_windows()
inserts a placeholder row with iid="__empty__", but other interaction paths
still act on that row; update _add_available_window_from_event() and
_show_available_context_menu() (and any handler that processes tree selections
for the available-tree) to check the selected item's iid and early-return when
iid == "__empty__" so placeholder rows are ignored consistently and no
actions/status updates are performed for the placeholder.
| if not self.draft_windows: | ||
| tree.insert( | ||
| "", | ||
| "end", | ||
| iid="__empty__", | ||
| values=( | ||
| "", | ||
| "No windows in layout draft. Add visible windows.", | ||
| "", | ||
| "", | ||
| "", | ||
| "", | ||
| ), | ||
| ) | ||
| for index, window in enumerate(self.draft_windows): |
There was a problem hiding this comment.
__empty__ row can crash draft context-menu selection via int(row_id) conversion.
After inserting iid="__empty__", _select_draft_window_from_event() still assumes numeric ids and does int(row_id). Right-click on the placeholder will raise ValueError.
Suggested fix
def _select_draft_window_from_event(self, event: tk.Event) -> int | None:
if not self._widget_alive(self.draft_tree):
return None
row_id = self.draft_tree.identify_row(event.y)
- if not row_id:
+ if not row_id or row_id == "__empty__":
return None
- index = int(row_id)
+ try:
+ index = int(row_id)
+ except ValueError:
+ return None
if not 0 <= index < len(self.draft_windows):
return None🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Window_Modern.py` around lines 5633 - 5647, The placeholder row is inserted
with iid="__empty__" which breaks _select_draft_window_from_event() because that
function assumes numeric row ids and calls int(row_id); modify the selection
handler to guard against the placeholder by checking for the "__empty__" iid (or
non-digit ids) before converting to int and return early (no selection) when
detected, or alternatively ensure the placeholder is inserted with a
numeric-but-invalid iid and handled accordingly; update references in
_select_draft_window_from_event() and where draft_windows are iterated to skip
or ignore the "__empty__" row.
1 participant
💡 What: Added empty state placeholder rows (
__empty__) to thedraft_treeandavailable_treeTreeviews when there are no items to display. Also updated interaction handlers to safely ignore clicks on these placeholder rows.🎯 Why: To improve UX and provide clear feedback when lists are empty, matching the existing standard established in
file_treeandrunning_tree.📸 Before/After: No images attached.
♿ Accessibility: Provides clear text feedback to the user instead of displaying a confusing blank table. Recorded learning in
.Jules/palette.md.PR created automatically by Jules for task 13273865375347327577 started by @agno7766
Summary by CodeRabbit