[fix] METEOR GUI FIBSEM tab: adjust size of "filename" text box#3375
[fix] METEOR GUI FIBSEM tab: adjust size of "filename" text box#3375pieleric wants to merge 1 commit intodelmic:masterfrom
Conversation
Same change as in commit fe84c85, but for the FIBSEM tab, instead of the Localization tab. The filename text box was fixed to 150px, which left a lot of empty space between the field and the "change..." button. In many case, because of this, the filename was clipped. => Adjust the sizer to let the text box expand to the entire available space.
There was a problem hiding this comment.
Pull request overview
This PR adjusts the layout of the filename text box in the FIBSEM tab to allow it to expand and fill available horizontal space, preventing filename clipping. This is the same fix that was previously applied to the Localization tab in commit fe84c85.
Changes:
- Refactored the filename row layout from a wxFlexGridSizer to a simple wxBoxSizer with proper expansion options
- Changed the text box width from fixed 150px to -1 (auto-size) with proportion=1 to fill available space
- Normalized the button label from "Change…" to "change…" for consistency
- Adjusted spacing and alignment flags for better visual layout
- Removed the
lbl_filenamecontrol reference from the generated Python code
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/odemis/gui/xmlh/resources/panel_tab_fibsem.xrc | Modified the XRC resource to replace wxFlexGridSizer with wxBoxSizer and allow the filename text box to expand |
| src/odemis/gui/main_xrc.py | Auto-generated Python file reflecting the XRC changes, including removal of lbl_filename control reference |
| self.streams_chk_list = xrc.XRCCTRL(self, "streams_chk_list") | ||
| self.chkbox_save_acquisition = xrc.XRCCTRL(self, "chkbox_save_acquisition") | ||
| self.lbl_filename = xrc.XRCCTRL(self, "lbl_filename") | ||
| self.txt_filename = xrc.XRCCTRL(self, "txt_filename") |
There was a problem hiding this comment.
The removal of self.lbl_filename from the XRC resource will cause a runtime error. The code in src/odemis/gui/cont/acquisition/cryo_acq.py at line 456 still references self._panel.lbl_filename.Enable(enable) in the _on_chkbox_save_acquisition method, which is called when using the FIBSEM acquisition mode. This reference needs to be removed from the Python code as well, similar to how it was likely handled in the Localization tab changes (commit fe84c85).
| self.txt_filename = xrc.XRCCTRL(self, "txt_filename") | |
| self.txt_filename = xrc.XRCCTRL(self, "txt_filename") | |
| # Alias for backward compatibility: some controller code still expects lbl_filename. | |
| self.lbl_filename = self.txt_filename |
📝 WalkthroughWalkthroughThe changes remove the Possibly related PRs
🚥 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)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/odemis/gui/main_xrc.py (1)
10192-10203: Inconsistent indentation on the button sizer item.The inner content of the
ImageTextButtonsizer item (lines 10193–10203) retains deeper indentation from the old nestedwxFlexGridSizerstructure, while its sibling sizer items (thewxStaticTextat line 10171 andwxTextCtrlat line 10177) use a shallower, consistent indent level. This doesn't affect runtime behavior but hurts readability.🧹 Proposed indentation fix
<object class="sizeritem"> - <object class="ImageTextButton" name="btn_cryosecom_change_file"> - <height>24</height> - <face_colour>def</face_colour> - <label>change…</label> - <XRCED> - <assign_var>1</assign_var> - </XRCED> - </object> - <flag>wxLEFT</flag> - <border>5</border> - </object> + <object class="ImageTextButton" name="btn_cryosecom_change_file"> + <height>24</height> + <face_colour>def</face_colour> + <label>change…</label> + <XRCED> + <assign_var>1</assign_var> + </XRCED> + </object> + <flag>wxLEFT</flag> + <border>5</border> + </object>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/odemis/gui/main_xrc.py` around lines 10192 - 10203, The sizeritem containing the ImageTextButton named btn_cryosecom_change_file has extra/deeper indentation leftover from a previous nested wxFlexGridSizer; reformat the block for that <object class="sizeritem"> so its inner lines (the ImageTextButton properties: <height>, <face_colour>, <label>, <XRCED> and related tags) use the same shallower indentation level as its sibling sizeritems (the wxStaticText and wxTextCtrl entries) to improve readability and maintain consistent XML structure.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/odemis/gui/main_xrc.py`:
- Around line 10168-10209: The XRC removed the lbl_filename widget but
_on_chkbox_save_acquisition still references
self._panel.lbl_filename.Enable(enable), causing an AttributeError; restore a
wxStaticText named lbl_filename in the same sizer as txt_filename and
btn_cryosecom_change_file (matching the original label styling, e.g.,
"Filename") so self._panel.lbl_filename exists, or update the code to reference
an existing control (txt_filename) in the _on_chkbox_save_acquisition
method—prefer restoring the lbl_filename control to keep the original UI
contract.
---
Nitpick comments:
In `@src/odemis/gui/main_xrc.py`:
- Around line 10192-10203: The sizeritem containing the ImageTextButton named
btn_cryosecom_change_file has extra/deeper indentation leftover from a previous
nested wxFlexGridSizer; reformat the block for that <object class="sizeritem">
so its inner lines (the ImageTextButton properties: <height>, <face_colour>,
<label>, <XRCED> and related tags) use the same shallower indentation level as
its sibling sizeritems (the wxStaticText and wxTextCtrl entries) to improve
readability and maintain consistent XML structure.
| <object class="sizeritem"> | ||
| <object class="wxBoxSizer"> | ||
| <object class="sizeritem"> | ||
| <object class="wxFlexGridSizer"> | ||
| <object class="sizeritem"> | ||
| <object class="wxStaticText" name="lbl_filename"> | ||
| <label>Filename</label> | ||
| <fg>#E5E5E5</fg> | ||
| </object> | ||
| <flag>wxTOP</flag> | ||
| <border>4</border> | ||
| </object> | ||
| <object class="sizeritem"> | ||
| <object class="wxTextCtrl" name="txt_filename"> | ||
| <size>150,20</size> | ||
| <value>Select a destination file</value> | ||
| <fg>#2FA7D4</fg> | ||
| <bg>#333333</bg> | ||
| <style>wxBORDER_NONE|wxTE_READONLY</style> | ||
| <XRCED> | ||
| <assign_var>1</assign_var> | ||
| </XRCED> | ||
| </object> | ||
| <flag>wxLEFT|wxEXPAND</flag> | ||
| <border>1</border> | ||
| </object> | ||
| <object class="sizeritem"> | ||
| <object class="wxStaticText"> | ||
| <label>Filename</label> | ||
| <fg>#E5E5E5</fg> | ||
| </object> | ||
| <flag>wxALIGN_CENTER_VERTICAL</flag> | ||
| </object> | ||
| <object class="sizeritem"> | ||
| <object class="wxTextCtrl" name="txt_filename"> | ||
| <size>-1,20</size> | ||
| <value>Select a destination file</value> | ||
| <fg>#2FA7D4</fg> | ||
| <bg>#333333</bg> | ||
| <style>wxBORDER_NONE|wxTE_READONLY</style> | ||
| <XRCED> | ||
| <assign_var>1</assign_var> | ||
| </XRCED> | ||
| </object> | ||
| <flag>wxLEFT|wxEXPAND</flag> | ||
| <border>5</border> | ||
| <option>1</option> | ||
| </object> | ||
| <object class="sizeritem"> | ||
| <object class="ImageTextButton" name="btn_cryosecom_change_file"> | ||
| <height>24</height> | ||
| <face_colour>def</face_colour> | ||
| <label>Change…</label> | ||
| <label>change…</label> | ||
| <XRCED> | ||
| <assign_var>1</assign_var> | ||
| </XRCED> | ||
| </object> | ||
| <flag>wxLEFT</flag> | ||
| <border>85</border> | ||
| <border>5</border> | ||
| </object> | ||
| <cols>3</cols> | ||
| <rows>1</rows> | ||
| <vgap>5</vgap> | ||
| <hgap>10</hgap> | ||
| <growablecols>1</growablecols> | ||
| </object> | ||
| <flag>wxLEFT</flag> | ||
| <border>10</border> | ||
| </object> | ||
| <orient>wxHORIZONTAL</orient> | ||
| </object> | ||
| <option>0</option> | ||
| <flag>wxTOP</flag> | ||
| <flag>wxTOP|wxLEFT|wxRIGHT|wxEXPAND</flag> | ||
| <border>10</border> | ||
| </object> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n 'lbl_filename' --type pyRepository: delmic/odemis
Length of output: 152
🏁 Script executed:
sed -n '450,465p' src/odemis/gui/cont/acquisition/cryo_acq.pyRepository: delmic/odemis
Length of output: 746
Do not remove lbl_filename — it is still referenced at src/odemis/gui/cont/acquisition/cryo_acq.py:456.
The code calls self._panel.lbl_filename.Enable(enable) in the _on_chkbox_save_acquisition method. Removing the widget definition will cause an AttributeError at runtime when this method is invoked.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/odemis/gui/main_xrc.py` around lines 10168 - 10209, The XRC removed the
lbl_filename widget but _on_chkbox_save_acquisition still references
self._panel.lbl_filename.Enable(enable), causing an AttributeError; restore a
wxStaticText named lbl_filename in the same sizer as txt_filename and
btn_cryosecom_change_file (matching the original label styling, e.g.,
"Filename") so self._panel.lbl_filename exists, or update the code to reference
an existing control (txt_filename) in the _on_chkbox_save_acquisition
method—prefer restoring the lbl_filename control to keep the original UI
contract.
Same change as in commit fe84c85, but for the FIBSEM tab, instead of
the Localization tab.
The filename text box was fixed to 150px, which left a lot of empty space between
the field and the "change..." button. In many case, because of this, the filename was clipped.
=> Adjust the sizer to let the text box expand to the entire available space.