[fix] NumberSlider height for itself and the linked_field

[nandishjpatel]

No description provided.

[Copilot]

Pull request overview

This PR adjusts the vertical sizing of the FastEM “Overview acquisition” settings area to better accommodate NumberSlider controls and their linked text fields, aiming to prevent clipping/misalignment in the UI.

Changes:

• Increase the pnl_overview_acq panel height from 100 to 102 in the XRC resource and its generated Python representation.
• Update the SettingsPanel instantiation size to match the new panel height.
• Modify NumberSlider sizing logic so the linked text field height follows the slider height, and update the slider’s minimum height calculation.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
src/odemis/gui/xmlh/resources/panel_tab_fastem_setup.xrc	Bumps the XRC-defined size for pnl_overview_acq to increase available vertical space.
src/odemis/gui/main_xrc.py	Mirrors the same panel size change in the generated XRC Python resource content.
src/odemis/gui/cont/acquisition/fastem_acq.py	Updates SettingsPanel(..., size=...) to align with the new panel height.
src/odemis/gui/comp/slider.py	Changes NumberSlider min-height logic and linked-field resizing behavior.

---

You can also share your feedback on Copilot code review. Take the survey.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a5888176-a0c0-4959-ad24-21f033d8faf5

📥 Commits

Reviewing files that changed from the base of the PR and between 76aa0c3 and c4363d4.

📒 Files selected for processing (1)

• src/odemis/gui/comp/slider.py

🚧 Files skipped from review as they are similar to previous changes (1)

• src/odemis/gui/comp/slider.py

---

📝 Walkthrough

Walkthrough

The changes update the NumberSlider class in src/odemis/gui/comp/slider.py. In __init__, the linked text field's minimum height is set using max(self.linked_field.GetCharHeight(), self.handle_height) (with the previous BestSize-based call left commented). In OnSize, the linked field's size is set with linked_field.SetSize((-1, height)), delegating width calculation to wx while enforcing the computed height.

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	❓ Inconclusive	No description was provided by the author, making it impossible to evaluate relevance to the changeset.	Add a pull request description explaining the motivation, problem being solved, or any relevant context for the height-related changes.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: fixing NumberSlider height for itself and the linked_field by adjusting size calculations.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with 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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[pieleric]

That's the core of this change. Apparently, BestSize returns a too large value with wxPython 4.2.1 on Ubuntu 24.04. Note that the wxPython 4.2.5 from pypi doesn't have this issue. So I think it'd be worthy to explain in a comment or at least in the commit text.
Something like "This [blabla] doesn't work [...], so we guess the best size based on the char height".
BTW, I've tried GetCharHeight() + 2 and that seems to give a bit more "room to breath" without being too high.

[nandishjpatel]

Just using GetCharHeight() for FASTEM

Using GetCharHeight() + 2 for FASTEM

@tepals do we need the +2 visually? then I have to add a few more pixels to the overview acq panel size as the Load button is cropped

[tepals]

We don't need the +2, but I think you do need to add a few more pixels to the overview acq panel size. This is what it looks like for me without the +2, you can see that the bottom few pixels of the load button and overview image text are cropped

[nandishjpatel]

Thanks, for few more pixels to the overview acq panel size comment, I will open a separate PR to automatically calculate the size for all FASTEM to make it future proof.

[pieleric]

I don't think that's really necessary, on my tests, it just moved the box up and down. Let's leave it as-is, to reduce chances of unexpected regressions on other platforms?
BTW, if you change the line, you can use -1 as first param, and drop the tuple, like that:
self.linked_field.SetSize(-1, self.GetHeight())

[nandishjpatel]

Okay, I leave it as it is

For FASTEM, using self.linked_field.SetSize((-1, self.GetHeight() + 4))