feat: add labels to all the inputs fields #53
Conversation
WalkthroughAdds a new LabeledInput UI component and refactors SocialEntry to use it for four fields. Updates on_list_view_selected to read selected filename from item_text_widget.content instead of item_text_widget.renderable. Also bumps project version in pyproject.toml. All changes are in src/edit_python_pe/main.py and pyproject.toml. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant SocialEntry
participant LabeledInput
participant Input as InternalInput
participant ListViewItem
Note over LabeledInput,Input: New component: label + input, .value proxy
User->>SocialEntry: Open form / edit fields
SocialEntry->>LabeledInput: Instantiate(label, placeholder, value)
LabeledInput->>Input: Create internal Input
User->>Input: Type text
Input-->>LabeledInput: on_change (text)
LabeledInput-->>SocialEntry: .value reflects input
User->>ListViewItem: Select file item
ListViewItem-->>SocialEntry: item_text_widget.content (used for filename)
Note right of SocialEntry: Uses .content instead of .renderable
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks (1 passed, 1 warning, 1 inconclusive)❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
✨ Finishing Touches🧪 Generate unit tests
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: 2
🧹 Nitpick comments (1)
src/edit_python_pe/main.py (1)
29-41: Optional: Style the provided.labelclass or remove it
You addclasses="label"but no CSS targets it. Minor polish.Apply:
@@ LabeledInput Input { width: 75%; } + LabeledInput .label { + text-align: right; + padding-right: 1; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/edit_python_pe/main.py(3 hunks)
🔇 Additional comments (2)
src/edit_python_pe/main.py (2)
172-183: Nice refactor to labeled fields
Clearer UX; placeholders preserved. Once LabeledInput.value is fixed, save flow should work as expected.
284-285: Keep compatibility with Textual’s Static API (content vs renderable)
Guard for both attributes to avoid breakage across versions.Apply:
- filename = item_text_widget.content + filename = getattr(item_text_widget, "content", None) + if filename is None: + filename = getattr(item_text_widget, "renderable", "") + if not isinstance(filename, str): + filename = str(filename)
| class LabeledInput(Vertical): | ||
|
|
||
| DEFAULT_CSS = """ | ||
| LabeledInput { | ||
| height: auto; | ||
| } | ||
| LabeledInput Static { | ||
| width: 25%; | ||
| } | ||
| LabeledInput Input { | ||
| width: 100%; | ||
| } | ||
| """ |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Side-by-side label/input won’t happen with Vertical; use Horizontal (or grid)
Current container stacks children; the widths won’t place them inline.
Apply:
- class LabeledInput(Vertical):
+ class LabeledInput(Horizontal):
@@
- DEFAULT_CSS = """
- LabeledInput {
- height: auto;
-
- }
+ DEFAULT_CSS = """
+ LabeledInput {
+ height: auto;
+ }
LabeledInput Static {
width: 25%;
}
LabeledInput Input {
- width: 100%;
+ width: 75%;
}
"""📝 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.
| class LabeledInput(Vertical): | |
| DEFAULT_CSS = """ | |
| LabeledInput { | |
| height: auto; | |
| } | |
| LabeledInput Static { | |
| width: 25%; | |
| } | |
| LabeledInput Input { | |
| width: 100%; | |
| } | |
| """ | |
| class LabeledInput(Horizontal): | |
| DEFAULT_CSS = """ | |
| LabeledInput { | |
| height: auto; | |
| } | |
| LabeledInput Static { | |
| width: 25%; | |
| } | |
| LabeledInput Input { | |
| width: 75%; | |
| } | |
| """ |
🤖 Prompt for AI Agents
In src/edit_python_pe/main.py around lines 27 to 41, the LabeledInput currently
subclasses Vertical which stacks children vertically so the label and input
widths will never sit side-by-side; change the container to Horizontal (or a
Grid) so children can be laid out inline, update the class inheritance from
Vertical to Horizontal (or Grid) and adjust DEFAULT_CSS selectors if needed
(e.g., target Horizontal Static and Input or grid columns) so the label receives
~25% width and the input 75%/100% of remaining space; ensure any spacing/padding
rules still apply for proper alignment.
| def compose(self) -> ComposeResult: | ||
| yield Static(self.label, classes="label") | ||
| yield Input( | ||
| id="input", placeholder=self.placeholder, value=self._value | ||
| ) | ||
|
|
||
| @property | ||
| def value(self): | ||
| return self._value | ||
|
|
||
| @value.setter | ||
| def value(self, v: str): | ||
| self._value = v | ||
| self.query_one(selector="#input").value = v | ||
|
|
There was a problem hiding this comment.
Fix value desync: LabeledInput.value doesn’t reflect user edits, causing saves to miss typed data
Getter returns the cached _value instead of the live Input.value. Setter relies on a DOM id lookup. This will persist empty strings even after typing.
Apply:
@@
- def compose(self) -> ComposeResult:
- yield Static(self.label, classes="label")
- yield Input(
- id="input", placeholder=self.placeholder, value=self._value
- )
+ def compose(self) -> ComposeResult:
+ yield Static(self.label, classes="label")
+ self._input = Input(placeholder=self.placeholder, value=self._value)
+ yield self._input
@@
- @property
- def value(self):
- return self._value
+ @property
+ def value(self) -> str:
+ # Reflect current field contents
+ return self._input.value if hasattr(self, "_input") else self._value
@@
- @value.setter
- def value(self, v: str):
- self._value = v
- self.query_one(selector="#input").value = v
+ @value.setter
+ def value(self, v: str) -> None:
+ if hasattr(self, "_input"):
+ self._input.value = v
+ self._value = v📝 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) -> ComposeResult: | |
| yield Static(self.label, classes="label") | |
| yield Input( | |
| id="input", placeholder=self.placeholder, value=self._value | |
| ) | |
| @property | |
| def value(self): | |
| return self._value | |
| @value.setter | |
| def value(self, v: str): | |
| self._value = v | |
| self.query_one(selector="#input").value = v | |
| def compose(self) -> ComposeResult: | |
| yield Static(self.label, classes="label") | |
| self._input = Input(placeholder=self.placeholder, value=self._value) | |
| yield self._input | |
| @property | |
| def value(self) -> str: | |
| # Reflect current field contents | |
| return self._input.value if hasattr(self, "_input") else self._value | |
| @value.setter | |
| def value(self, v: str) -> None: | |
| if hasattr(self, "_input"): | |
| self._input.value = v | |
| self._value = v |
🤖 Prompt for AI Agents
In src/edit_python_pe/main.py around lines 51 to 65, the LabeledInput.value
getter returns the cached _value and the setter updates the DOM via a brittle id
lookup, causing desync with user edits; change the getter to return the live
Input widget value (query the Input widget and return its .value if present,
falling back to _value) and update the setter to set both the internal _value
and the Input widget's .value using a typed widget lookup (handle the case where
the widget isn't mounted to avoid exceptions) so in-memory state and the
displayed input always stay in sync.
2 participants
Summary by CodeRabbit
New Features
UI Improvements
Bug Fixes
Chores