Enhance TUI visuals and UX #1
Conversation
- Redesign CSS with Firefoo-inspired dark theme - Add spinner/loading widget for async operations - Improve help screen with better styling and keyboard hints - Add better empty states and loading indicators in document table - Enhance JSON viewer with type summaries and improved syntax highlighting - Reduce panel heights for cleaner layout - Add hover effects and focus states for better keyboard navigation
📝 WalkthroughWalkthroughRedesign of the Fayastore CLI TUI: new dark theme and style rules, revamped help screen, added Spinner/LoadingOverlay widgets, enhanced DocumentTable (loading/empty/error/filter states) and JsonViewer (themes, document summary, empty handling). Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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: 5
🤖 Fix all issues with AI agents
In `@src/fayastore_cli/tui/widgets/document_table.py`:
- Around line 54-72: The load_collection method currently defaults
show_loading=True which causes an early return and prevents documents from being
loaded; change the default parameter in load_collection(collection_path: str,
limit: int = 50, show_loading: bool = False) so the function proceeds to load
documents by default, keep the existing early-return behavior only when
show_loading is True, and update any callers that relied on the old behavior to
explicitly pass show_loading=True when they only want the loading state (e.g.,
the call site in the browser module).
- Around line 207-218: In refresh_rows, when adding rows from self._documents
the fallback key uses str(len(row_values)) which is identical for every row;
change the loop to enumerate over self._documents and pass a unique fallback key
(e.g. include the index) to add_row when doc.get("id") is missing so each row
key is unique; locate refresh_rows and modify the for doc in self._documents
loop to for index, doc in enumerate(self._documents) and use doc.get("id",
f"row-{index}") (or similar) as the key for add_row to ensure unique row
identification.
- Around line 189-203: In filter_documents, when filtered is empty you currently
call clear(rows=True) then add_row with only two cells which will error if the
table has more columns; update the empty-state row creation in filter_documents
to construct a list of cell values sized to the current table column count (use
the table's column list or column_count property) and pad the remaining cells
with empty Text() or dim placeholders before calling add_row; reference the
filter_documents method, the set_empty_state call, clear(rows=True), and the
add_row invocation to implement the padding so the added row always matches the
table's column count.
In `@src/fayastore_cli/tui/widgets/spinner.py`:
- Around line 62-68: The compose method in LoadingOverlay is trying to
concatenate a Textual widget with strings which raises a TypeError; replace the
expression using the local Spinner widget instead of textual.widgets.Spinner and
compose the content as a widget tree (e.g., Spinner() plus a separate Textual
text widget or include the message as Spinner’s property) so that
LoadingOverlay.compose yields valid Widget children; update references in
LoadingOverlay.compose (currently using TextualSpinner(), self._message, and
classes="loading-content") to use the local Spinner class rather than
concatenating widgets and strings.
- Around line 22-48: The spinner never starts because on_mount() doesn't create
a Timer; modify on_mount() to start an interval (use self.set_interval or
self.set_timer) that calls the existing on_timer callback and set an internal
handle/ID (e.g., self._timer) and set a boolean flag (e.g., self._animating =
True); change is_animating to return that boolean instead of checking
_frame_index; update stop() to cancel the timer via the stored handle (or call
self.remove_timer) and set self._animating = False, update the displayed
checkmark, and optionally reset _frame_index if you want the next start to begin
from frame 0.
🧹 Nitpick comments (3)
src/fayastore_cli/tui/widgets/spinner.py (1)
11-11: Consider ClassVar (or tuple) forSPINNER_FRAMES.Helps satisfy Ruff’s RUF012 and documents immutability intent.
♻️ Suggested tweak
-from typing import Optional +from typing import ClassVar, Optional @@ - SPINNER_FRAMES = ["⠋", "⠙", "⠹", "⠸", "⠼", "⠴", "⠦", "⠧", "⠇", "⠏"] + SPINNER_FRAMES: ClassVar[tuple[str, ...]] = ( + "⠋", "⠙", "⠹", "⠸", "⠼", "⠴", "⠦", "⠧", "⠇", "⠏" + )src/fayastore_cli/tui/widgets/document_table.py (1)
27-32: MarkEMPTY_STATESas ClassVar (Ruff RUF012).Helps linting and clarifies this is a shared constant.
♻️ Suggested tweak
-from typing import TYPE_CHECKING, Any, Dict, List, Optional +from typing import TYPE_CHECKING, Any, ClassVar, Dict, List, Optional @@ - EMPTY_STATES = { + EMPTY_STATES: ClassVar[dict[str, str]] = { "default": "No documents in this collection", "loading": "Loading documents...", "error": "Error loading documents", "filtered": "No documents match your filter", }src/fayastore_cli/tui/widgets/json_viewer.py (1)
17-21: Optional lint cleanups for constants and unused loop var.Ruff will flag the mutable class attr and unused
keyinget_summary.♻️ Suggested tweak
-from typing import Any, Dict, Optional +from typing import Any, ClassVar, Dict, Optional @@ - THEMES = { + THEMES: ClassVar[dict[str, str]] = { "dark": "monokai", "light": "github-light", "auto": "auto", } @@ - for key, value in self._data.items(): + for _key, value in self._data.items(): type_name = type(value).__name__ type_counts[type_name] = type_counts.get(type_name, 0) + 1Also applies to: 108-113
| def load_collection( | ||
| self, | ||
| collection_path: str, | ||
| limit: int = 50, | ||
| show_loading: bool = True, | ||
| ) -> None: | ||
| """Load documents from a collection.""" | ||
| self._current_collection = collection_path | ||
| self.clear(columns=True) | ||
| self._documents = [] | ||
|
|
||
| if show_loading: | ||
| self.set_empty_state("loading") | ||
| self.clear(columns=True) | ||
| self.add_column("Status", key="status") | ||
| self.add_row( | ||
| Text(self.EMPTY_STATES["loading"], style="italic dim") | ||
| ) | ||
| return | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, read the full file to understand the complete method
wc -l src/fayastore_cli/tui/widgets/document_table.pyRepository: nbckk9/fayastore-cli
Length of output: 115
🏁 Script executed:
# Read the relevant section and some context to understand the full method
sed -n '40,100p' src/fayastore_cli/tui/widgets/document_table.pyRepository: nbckk9/fayastore-cli
Length of output: 2175
🏁 Script executed:
# Find all call sites of load_collection
rg -n "load_collection\s*\(" --type py -A 3 -B 1Repository: nbckk9/fayastore-cli
Length of output: 1338
🏁 Script executed:
# Check browser.py around the load_collection call
sed -n '70,90p' src/fayastore_cli/tui/screens/browser.pyRepository: nbckk9/fayastore-cli
Length of output: 919
🏁 Script executed:
# Search for any event handlers or callbacks that might trigger a follow-up load
rg -n "show_loading\s*=\s*False" --type py -B 3 -A 1Repository: nbckk9/fayastore-cli
Length of output: 527
Change default show_loading to False to actually load documents.
The current default of show_loading=True causes the method to return early after setting a loading state, without loading any documents. In browser.py line 78, the call uses this default and has no follow-up with show_loading=False, leaving the table stuck showing "loading" while document_count remains 0. The status update on line 79 is then misleading since no documents were actually loaded.
- limit: int = 50,
- show_loading: bool = True,
+ limit: int = 50,
+ show_loading: bool = False,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def load_collection( | |
| self, | |
| collection_path: str, | |
| limit: int = 50, | |
| show_loading: bool = True, | |
| ) -> None: | |
| """Load documents from a collection.""" | |
| self._current_collection = collection_path | |
| self.clear(columns=True) | |
| self._documents = [] | |
| if show_loading: | |
| self.set_empty_state("loading") | |
| self.clear(columns=True) | |
| self.add_column("Status", key="status") | |
| self.add_row( | |
| Text(self.EMPTY_STATES["loading"], style="italic dim") | |
| ) | |
| return | |
| def load_collection( | |
| self, | |
| collection_path: str, | |
| limit: int = 50, | |
| show_loading: bool = False, | |
| ) -> None: | |
| """Load documents from a collection.""" | |
| self._current_collection = collection_path | |
| self._documents = [] | |
| if show_loading: | |
| self.set_empty_state("loading") | |
| self.clear(columns=True) | |
| self.add_column("Status", key="status") | |
| self.add_row( | |
| Text(self.EMPTY_STATES["loading"], style="italic dim") | |
| ) | |
| return |
🤖 Prompt for AI Agents
In `@src/fayastore_cli/tui/widgets/document_table.py` around lines 54 - 72, The
load_collection method currently defaults show_loading=True which causes an
early return and prevents documents from being loaded; change the default
parameter in load_collection(collection_path: str, limit: int = 50,
show_loading: bool = False) so the function proceeds to load documents by
default, keep the existing early-return behavior only when show_loading is True,
and update any callers that relied on the old behavior to explicitly pass
show_loading=True when they only want the loading state (e.g., the call site in
the browser module).
| def filter_documents(self, filter_fn) -> None: | ||
| """Filter documents based on a function.""" | ||
| if not self._documents: | ||
| return | ||
|
|
||
| filtered = [doc for doc in self._documents if filter_fn(doc)] | ||
| self._documents = filtered | ||
|
|
||
| if not filtered: | ||
| self.set_empty_state("filtered") | ||
| self.clear(rows=True) | ||
| self.add_row( | ||
| Text("🔍 No matching documents", style="dim italic"), | ||
| Text("Try a different search term.", style="dim"), | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/fayastore_cli/tui/widgets/document_table.py | head -250Repository: nbckk9/fayastore-cli
Length of output: 9012
🏁 Script executed:
rg "add_row" src/fayastore_cli/tui/widgets/document_table.py -A 2 -B 2Repository: nbckk9/fayastore-cli
Length of output: 1438
🏁 Script executed:
rg "self.columns" src/fayastore_cli/tui/widgets/document_table.py -A 1 -B 1Repository: nbckk9/fayastore-cli
Length of output: 172
🏁 Script executed:
rg "class DocumentTable" src/fayastore_cli/tui/widgets/document_table.py -A 30 | head -50Repository: nbckk9/fayastore-cli
Length of output: 1123
🏁 Script executed:
python3 << 'EOF'
# Let's search for DataTable usage and understand how it handles cell/column mismatches
import subprocess
# First, check if there's any error handling or test files that mention DataTable errors
result = subprocess.run(['rg', 'DataTable', 'src/', '-l'], capture_output=True, text=True)
print("Files mentioning DataTable:")
print(result.stdout)
# Also check for any tests
result2 = subprocess.run(['find', 'tests/', '-name', '*test*.py', '-o', '-name', '*_test.py'],
capture_output=True, text=True, errors='ignore')
print("\nTest files:")
print(result2.stdout if result2.stdout else "No test files found in tests/")
EOFRepository: nbckk9/fayastore-cli
Length of output: 222
🏁 Script executed:
cat -n src/fayastore_cli/tui/screens/query.py | head -100Repository: nbckk9/fayastore-cli
Length of output: 4281
🏁 Script executed:
# Let's check the imports and understand the textual version requirements
rg "textual" . --include="*.txt" --include="*.toml" --include="*.cfg" --include="pyproject.toml" -A 2Repository: nbckk9/fayastore-cli
Length of output: 505
🏁 Script executed:
fd "pyproject.toml|requirements.txt|setup.py" . -x cat {}Repository: nbckk9/fayastore-cli
Length of output: 660
🏁 Script executed:
# Let's check if there are any error messages or issues in the codebase related to DataTable
rg "add_row|columns" src/fayastore_cli/tui/widgets/document_table.py -nRepository: nbckk9/fayastore-cli
Length of output: 944
🏁 Script executed:
# Let's trace the control flow to see if filter_documents can be called after loading documents
rg "filter_documents" src/ -B 3 -A 3Repository: nbckk9/fayastore-cli
Length of output: 670
🏁 Script executed:
rg "\.filter_documents" src/ -B 3 -A 3Repository: nbckk9/fayastore-cli
Length of output: 46
🏁 Script executed:
# Search for any calls to filter_documents
rg "filter_documents" src/ -B 5 -A 5Repository: nbckk9/fayastore-cli
Length of output: 1020
🏁 Script executed:
# Let's verify the actual flow - check where filter_documents might be called from
rg "filter_documents|search" src/fayastore_cli/tui/screens/ -B 2 -A 2Repository: nbckk9/fayastore-cli
Length of output: 936
🏁 Script executed:
cat -n src/fayastore_cli/tui/screens/browser.py | grep -A 50 "def action_search"Repository: nbckk9/fayastore-cli
Length of output: 790
🏁 Script executed:
# Let me check all the places where add_row is called and see the pattern of column/cell matching
python3 << 'EOF'
# Analyze the add_row calls and column setup patterns
import re
with open('src/fayastore_cli/tui/widgets/document_table.py', 'r') as f:
content = f.read()
lines = content.split('\n')
# Find all add_row calls
for i, line in enumerate(lines, 1):
if 'add_row' in line:
# Show context
start = max(0, i-5)
end = min(len(lines), i+4)
print(f"\n--- Line {i}: add_row call ---")
for j in range(start, end):
prefix = ">>> " if j == i-1 else " "
print(f"{prefix}{j+1}: {lines[j]}")
EOFRepository: nbckk9/fayastore-cli
Length of output: 2634
🏁 Script executed:
# Let me trace the actual column count scenario when filter_documents is called
python3 << 'EOF'
# Simulate the flow
print("=== Flow Analysis ===\n")
print("SCENARIO 1: User loads a collection with multiple documents")
print("- load_collection() is called")
print("- Documents are analyzed, and up to 8 columns are created (lines 94-104)")
print("- Documents are added as rows (line 112)")
print("")
print("SCENARIO 2: User performs a search/filter")
print("- filter_documents() is called")
print("- clear(rows=True) is called (line 199) - only clears rows, NOT columns")
print("- The 8 columns from load_collection still exist!")
print("- add_row() is called with only 2 cells (lines 200-203)")
print("- MISMATCH: 8 columns but only 2 cells provided")
print("")
print("EXPECTED ERROR from Textual DataTable:")
print("DataTable enforces that each row has exactly the same number of cells as columns")
print("This would likely raise: ValueError or similar during add_row()")
EOFRepository: nbckk9/fayastore-cli
Length of output: 775
Filtered empty-state row must pad cells to match column count.
The clear(rows=True) call preserves existing columns from the previous data load (up to 8 columns from line 97), but only 2 cells are provided in the add_row() call. This causes a cell count mismatch that will raise an error when the table has more than 2 columns.
🛠️ Pad row to current column count
if not filtered:
self.set_empty_state("filtered")
self.clear(rows=True)
- self.add_row(
- Text("🔍 No matching documents", style="dim italic"),
- Text("Try a different search term.", style="dim"),
- )
+ row = [
+ Text("🔍 No matching documents", style="dim italic"),
+ Text("Try a different search term.", style="dim"),
+ ]
+ if len(self.columns) > 2:
+ row.extend([Text("")] * (len(self.columns) - 2))
+ self.add_row(*row)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def filter_documents(self, filter_fn) -> None: | |
| """Filter documents based on a function.""" | |
| if not self._documents: | |
| return | |
| filtered = [doc for doc in self._documents if filter_fn(doc)] | |
| self._documents = filtered | |
| if not filtered: | |
| self.set_empty_state("filtered") | |
| self.clear(rows=True) | |
| self.add_row( | |
| Text("🔍 No matching documents", style="dim italic"), | |
| Text("Try a different search term.", style="dim"), | |
| ) | |
| def filter_documents(self, filter_fn) -> None: | |
| """Filter documents based on a function.""" | |
| if not self._documents: | |
| return | |
| filtered = [doc for doc in self._documents if filter_fn(doc)] | |
| self._documents = filtered | |
| if not filtered: | |
| self.set_empty_state("filtered") | |
| self.clear(rows=True) | |
| row = [ | |
| Text("🔍 No matching documents", style="dim italic"), | |
| Text("Try a different search term.", style="dim"), | |
| ] | |
| if len(self.columns) > 2: | |
| row.extend([Text("")] * (len(self.columns) - 2)) | |
| self.add_row(*row) |
🤖 Prompt for AI Agents
In `@src/fayastore_cli/tui/widgets/document_table.py` around lines 189 - 203, In
filter_documents, when filtered is empty you currently call clear(rows=True)
then add_row with only two cells which will error if the table has more columns;
update the empty-state row creation in filter_documents to construct a list of
cell values sized to the current table column count (use the table's column list
or column_count property) and pad the remaining cells with empty Text() or dim
placeholders before calling add_row; reference the filter_documents method, the
set_empty_state call, clear(rows=True), and the add_row invocation to implement
the padding so the added row always matches the table's column count.
| def refresh_rows(self) -> None: | ||
| """Refresh table rows with current documents.""" | ||
| if not self._documents: | ||
| return | ||
|
|
||
| self.clear(rows=True) | ||
| for doc in self._documents: | ||
| row_values = [] | ||
| for col in self.columns.keys(): | ||
| value = doc.get(col) | ||
| row_values.append(self._format_cell_value(value)) | ||
| self.add_row(*row_values, key=doc.get("id", str(len(row_values)))) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if the file exists and read the relevant section
cat -n src/fayastore_cli/tui/widgets/document_table.py | sed -n '200,225p'Repository: nbckk9/fayastore-cli
Length of output: 885
🏁 Script executed:
# Search for where columns are defined in this class
rg -A 5 -B 5 "self\.columns" src/fayastore_cli/tui/widgets/document_table.py | head -60Repository: nbckk9/fayastore-cli
Length of output: 403
🏁 Script executed:
# Check how row keys are used elsewhere in the codebase
rg "add_row" src/fayastore_cli/tui/widgets/document_table.py -A 2 -B 2Repository: nbckk9/fayastore-cli
Length of output: 1438
🏁 Script executed:
# Check document structure and initialization to understand if "id" is guaranteed
rg "class.*Document\|def.*document\|_documents.*=" src/fayastore_cli/tui/widgets/document_table.py -B 3 -A 3 | head -40Repository: nbckk9/fayastore-cli
Length of output: 46
🏁 Script executed:
# Search for how documents are fetched/created to see if id is always present
rg "get.*document\|fetch.*document" src/fayastore_cli --type py -B 2 -A 2 | head -50Repository: nbckk9/fayastore-cli
Length of output: 46
🏁 Script executed:
# Look for _documents initialization and class definition
rg "_documents" src/fayastore_cli/tui/widgets/document_table.py -B 5 -A 2Repository: nbckk9/fayastore-cli
Length of output: 3149
🏁 Script executed:
# Check the class definition to understand the widget type and row key handling
head -100 src/fayastore_cli/tui/widgets/document_table.pyRepository: nbckk9/fayastore-cli
Length of output: 3264
Duplicate fallback row keys will break selection and updates.
str(len(row_values)) is identical for every row since the column count is static. When documents lack an id field, all rows receive the same key, preventing proper row identification and selection.
Use enumerate to generate unique fallback keys:
Unique fallback key
- for doc in self._documents:
+ for idx, doc in enumerate(self._documents):
@@
- self.add_row(*row_values, key=doc.get("id", str(len(row_values))))
+ self.add_row(*row_values, key=doc.get("id", str(idx)))🤖 Prompt for AI Agents
In `@src/fayastore_cli/tui/widgets/document_table.py` around lines 207 - 218, In
refresh_rows, when adding rows from self._documents the fallback key uses
str(len(row_values)) which is identical for every row; change the loop to
enumerate over self._documents and pass a unique fallback key (e.g. include the
index) to add_row when doc.get("id") is missing so each row key is unique;
locate refresh_rows and modify the for doc in self._documents loop to for index,
doc in enumerate(self._documents) and use doc.get("id", f"row-{index}") (or
similar) as the key for add_row to ensure unique row identification.
| def on_mount(self) -> None: | ||
| """Start the animation.""" | ||
| self._update_content() | ||
|
|
||
| def on_timer(self) -> None: | ||
| """Update the spinner frame.""" | ||
| self._frame_index = (self._frame_index + 1) % len(self.SPINNER_FRAMES) | ||
| self._update_content() | ||
|
|
||
| def _update_content(self) -> None: | ||
| """Update the displayed content.""" | ||
| frame = self.SPINNER_FRAMES[self._frame_index] | ||
| self.update(f"{frame} {self._text}") | ||
|
|
||
| def set_text(self, text: str) -> None: | ||
| """Set the loading text.""" | ||
| self._text = text | ||
| self._update_content() | ||
|
|
||
| @property | ||
| def is_animating(self) -> bool: | ||
| """Check if the spinner is animating.""" | ||
| return self._frame_index > 0 | ||
|
|
||
| def stop(self) -> None: | ||
| """Stop the spinner animation.""" | ||
| self.update(f"✓ {self._text}") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "spinner.py" -path "*/tui/widgets/*"Repository: nbckk9/fayastore-cli
Length of output: 106
🏁 Script executed:
cat -n src/fayastore_cli/tui/widgets/spinner.pyRepository: nbckk9/fayastore-cli
Length of output: 2383
🏁 Script executed:
# Search for timer-related methods in the file
rg "set_interval|set_timer|on_timer|_timer" src/fayastore_cli/tui/widgets/spinner.py -A 2 -B 2Repository: nbckk9/fayastore-cli
Length of output: 246
🏁 Script executed:
# Search for any usage of the Spinner class
rg "Spinner" src/ --type python -B 2 -A 2Repository: nbckk9/fayastore-cli
Length of output: 94
🏁 Script executed:
# Check if there are any timer-related imports or mechanisms in the project
rg "set_interval|set_timer" src/ --type pythonRepository: nbckk9/fayastore-cli
Length of output: 94
🌐 Web query:
Textual Python framework on_timer lifecycle callback behavior
💡 Result:
In Textual, timers created via set_interval(...) / set_timer(...) don’t call on_timer directly. Instead they post a textual.events.Timer event to the timer’s event target (the App/Widget/other MessagePump that created it). That event is bubbling and verbose. [1]
What on_timer does (and when it runs)
- If your widget/app defines
on_timer(self, event: events.Timer), it will be invoked when theTimerevent is processed from the message queue (i.e., like any other event handler). [1]
How the callback= you pass to set_interval / set_timer relates
- The
Timerobject has an optionalcallback“to invoke when the event is handled”. In other words, the callback is tied to the timer event’s handling lifecycle (it is not a separate scheduler thread calling your function). [2] set_interval/set_timerboth accept that optional callback and return aTimeryou canpause(),resume(),reset(), orstop(). [3]
Lifecycle / teardown behavior
- If the timer’s event target is gone (deleted) before the timer event can be sent, Textual raises
EventTargetGone. [2] - Practically: start timers in
on_mount/on_ready, andstop()them (or rely on teardown) when the widget/app is unmounted/closing to avoid stray ticks. [2]
Sources:
[1] Textual events.Timer event docs (bubbling/verbose; “Sent in response to a timer”) (textual.textualize.io)
[2] Textual textual.timer.Timer docs (callback semantics; EventTargetGone) (textual.textualize.io)
[3] Textual MessagePump.set_interval / set_timer API docs (creation + callback parameter + returned Timer) (textual.textualize.io)
Citations:
- 1: https://textual.textualize.io/api/events/?utm_source=openai
- 2: https://textual.textualize.io/api/timer/?utm_source=openai
- 3: https://textual.textualize.io/api/message_pump/?utm_source=openai
🏁 Script executed:
rg "Spinner\(" src/ -A 2 -B 2Repository: nbckk9/fayastore-cli
Length of output: 683
🏁 Script executed:
# Also check for any imports of Spinner
rg "from.*spinner import|import.*spinner" src/Repository: nbckk9/fayastore-cli
Length of output: 150
🏁 Script executed:
# Search for actual usage of the custom Spinner class (not TextualSpinner)
rg "from.*widgets.*import.*Spinner" src/ -B 2 -A 5Repository: nbckk9/fayastore-cli
Length of output: 646
🏁 Script executed:
# Search all usages of 'Spinner' in the codebase
fd "\.py$" src/ | xargs grep -l "Spinner" | head -20Repository: nbckk9/fayastore-cli
Length of output: 219
🏁 Script executed:
# Look at the __init__.py to see what's exported
cat -n src/fayastore_cli/tui/widgets/__init__.pyRepository: nbckk9/fayastore-cli
Length of output: 632
🏁 Script executed:
# Check usage in batch.py
rg "Spinner|LoadingOverlay" src/fayastore_cli/commands/batch.py -B 2 -A 5Repository: nbckk9/fayastore-cli
Length of output: 1148
🏁 Script executed:
# Check usage in export.py
rg "Spinner|LoadingOverlay" src/fayastore_cli/commands/export.py -B 2 -A 5Repository: nbckk9/fayastore-cli
Length of output: 1134
🏁 Script executed:
# Check if Spinner is used elsewhere
rg "Spinner\(\)" src/ -B 3 -A 3Repository: nbckk9/fayastore-cli
Length of output: 495
🏁 Script executed:
# Search for any direct instantiation or reference to the custom Spinner class (not TextualSpinner)
rg "widgets\.Spinner|from.*spinner.*import.*Spinner" src/ --type-listRepository: nbckk9/fayastore-cli
Length of output: 453
🏁 Script executed:
# Broader search to see if Spinner is referenced in any non-shell way
find src -name "*.py" -exec grep -l "widgets.Spinner\|from .spinner import Spinner" {} \;Repository: nbckk9/fayastore-cli
Length of output: 46
🏁 Script executed:
# Check if LoadingOverlay is used anywhere (which is also exported alongside Spinner)
rg "LoadingOverlay" src/ -B 2 -A 2Repository: nbckk9/fayastore-cli
Length of output: 940
🏁 Script executed:
# Do a comprehensive search to see if custom Spinner is imported or used anywhere
find src -name "*.py" -exec grep -H "Spinner" {} \; | grep -v "TextualSpinner"Repository: nbckk9/fayastore-cli
Length of output: 861
🏁 Script executed:
# Check all Python files that might import from widgets
fd "\.py$" src/fayastore_cli --exec grep -l "from.*widgets import\|from.*tui import" {} \;Repository: nbckk9/fayastore-cli
Length of output: 514
🏁 Script executed:
# Look at all files in tui directory
find src/fayastore_cli/tui -name "*.py" -exec basename {} \;Repository: nbckk9/fayastore-cli
Length of output: 228
🏁 Script executed:
# Search for Spinner usage in the TUI screens and app
rg "Spinner|LoadingOverlay" src/fayastore_cli/tui/screens/ src/fayastore_cli/tui/app.py -B 2 -A 2Repository: nbckk9/fayastore-cli
Length of output: 46
🏁 Script executed:
# Check if the custom Spinner is ever actually instantiated in the codebase
grep -r "Spinner(" src/ --include="*.py" | grep -v "TextualSpinner\|SpinnerColumn"Repository: nbckk9/fayastore-cli
Length of output: 127
Spinner never starts animation—on_timer() callback is never triggered.
Textual's on_timer() is an event handler that fires only when a Timer event is posted via set_interval() or set_timer(). The on_mount() method does not start an interval, so on_timer() will never execute and the spinner will remain static. Additionally, stop() does not cancel any timer, and the is_animating property returns self._frame_index > 0, which is misleading since it reflects frame progression, not actual animation state.
🛠️ Possible fix (start/stop interval + accurate state)
class Spinner(Static):
@@
def __init__(
self,
text: str = "Loading...",
id: Optional[str] = None,
) -> None:
super().__init__(id=id)
self._text = text
self._frame_index = 0
+ self._timer = None
+ self._animating = False
@@
def on_mount(self) -> None:
"""Start the animation."""
self._update_content()
+ self._timer = self.set_interval(0.1, self.on_timer)
+ self._animating = True
@@
`@property`
def is_animating(self) -> bool:
"""Check if the spinner is animating."""
- return self._frame_index > 0
+ return self._animating
@@
def stop(self) -> None:
"""Stop the spinner animation."""
+ if self._timer:
+ self._timer.stop()
+ self._animating = False
self.update(f"✓ {self._text}")🤖 Prompt for AI Agents
In `@src/fayastore_cli/tui/widgets/spinner.py` around lines 22 - 48, The spinner
never starts because on_mount() doesn't create a Timer; modify on_mount() to
start an interval (use self.set_interval or self.set_timer) that calls the
existing on_timer callback and set an internal handle/ID (e.g., self._timer) and
set a boolean flag (e.g., self._animating = True); change is_animating to return
that boolean instead of checking _frame_index; update stop() to cancel the timer
via the stored handle (or call self.remove_timer) and set self._animating =
False, update the displayed checkmark, and optionally reset _frame_index if you
want the next start to begin from frame 0.
| def compose(self): | ||
| from textual.containers import Center | ||
| from textual.widgets import Spinner as TextualSpinner | ||
|
|
||
| yield Center( | ||
| TextualSpinner() + " " + self._message, | ||
| classes="loading-content", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/fayastore_cli/tui/widgets/spinner.pyRepository: nbckk9/fayastore-cli
Length of output: 2383
🏁 Script executed:
# Check if there's a requirements or setup file to see Textual version
find . -type f \( -name "requirements*.txt" -o -name "setup.py" -o -name "pyproject.toml" -o -name "poetry.lock" \) | head -5Repository: nbckk9/fayastore-cli
Length of output: 80
🏁 Script executed:
cat pyproject.tomlRepository: nbckk9/fayastore-cli
Length of output: 660
🏁 Script executed:
rg "LoadingOverlay" --type py -nRepository: nbckk9/fayastore-cli
Length of output: 293
🏁 Script executed:
rg "LoadingOverlay\(" --type py -B 2 -A 2Repository: nbckk9/fayastore-cli
Length of output: 342
🏁 Script executed:
# Search for any instantiation or usage of LoadingOverlay in the codebase
rg "LoadingOverlay" --type py -C 3Repository: nbckk9/fayastore-cli
Length of output: 1336
🏁 Script executed:
# Search more broadly for LoadingOverlay instantiation patterns
rg "LoadingOverlay\s*\(" --type py -B 2 -A 2Repository: nbckk9/fayastore-cli
Length of output: 342
🏁 Script executed:
# Search for imports of LoadingOverlay beyond the widget module
rg "from.*widgets.*import.*LoadingOverlay" --type py -B 1 -A 1Repository: nbckk9/fayastore-cli
Length of output: 46
🏁 Script executed:
# Check if there are any test files that might use LoadingOverlay
find . -type f -name "*test*.py" | xargs grep -l "LoadingOverlay" 2>/dev/null || echo "No test files found with LoadingOverlay"Repository: nbckk9/fayastore-cli
Length of output: 103
Fix widget composition in LoadingOverlay.compose()—concatenating string with Widget will raise TypeError.
The expression TextualSpinner() + " " + self._message is invalid; Textual widgets don't support concatenation with strings. Replace with the local Spinner class that already exists in this file:
Correct composition
def compose(self):
from textual.containers import Center
- from textual.widgets import Spinner as TextualSpinner
yield Center(
- TextualSpinner() + " " + self._message,
+ Spinner(text=self._message),
classes="loading-content",
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def compose(self): | |
| from textual.containers import Center | |
| from textual.widgets import Spinner as TextualSpinner | |
| yield Center( | |
| TextualSpinner() + " " + self._message, | |
| classes="loading-content", | |
| def compose(self): | |
| from textual.containers import Center | |
| yield Center( | |
| Spinner(text=self._message), | |
| classes="loading-content", |
🤖 Prompt for AI Agents
In `@src/fayastore_cli/tui/widgets/spinner.py` around lines 62 - 68, The compose
method in LoadingOverlay is trying to concatenate a Textual widget with strings
which raises a TypeError; replace the expression using the local Spinner widget
instead of textual.widgets.Spinner and compose the content as a widget tree
(e.g., Spinner() plus a separate Textual text widget or include the message as
Spinner’s property) so that LoadingOverlay.compose yields valid Widget children;
update references in LoadingOverlay.compose (currently using TextualSpinner(),
self._message, and classes="loading-content") to use the local Spinner class
rather than concatenating widgets and strings.
1 participant
Summary
Redesigned the TUI with a Firefoo-inspired dark theme for better usability.
Changes
Redesigned CSS (
styles.tcss)New Spinner Widget (
spinner.py)Improved Help Screen (
help.py)Enhanced Document Table (
document_table.py)Improved JSON Viewer (
json_viewer.py)Testing
Run the TUI to see the changes:
Notes
Summary by CodeRabbit
New Features
UI/UX Improvements