Skip to content

Conversation

@0tkl
Copy link
Contributor

@0tkl 0tkl commented Dec 5, 2025

Before:
before

After:
after

add_with_label(OutlineBox, _("Shadow:"), Shadow);
add_with_label(OutlineBox, _("Border style:"), OutlineType);
OutlineBox->Add(new wxStaticText(this, -1, _("Border style:")), wxSizerFlags().Center().HorzBorder());
OutlineBox->Add(OutlineType, wxSizerFlags().Left().Expand());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For consistency, I'd rather add an optional parameter proportion to add_with_label. As far as I can tell, the sizer flags for the label match here match the one in add_with_label, right?

@0tkl 0tkl marked this pull request as draft December 6, 2025 17:07
@0tkl
Copy link
Contributor Author

0tkl commented Dec 6, 2025

After thinking for a while longer, I considered deeper questions which confused me again. What behavior do we expect the dialog box to exhibit when it is "stretched and widened"? I feel that the behavior prior to this PR, scaling these two spinboxes proportionally to the combobox, is reasonable.

@0tkl 0tkl marked this pull request as ready for review December 6, 2025 17:59
@0tkl
Copy link
Contributor Author

0tkl commented Dec 6, 2025

I was barking up the wrong tree :-(

@0tkl 0tkl force-pushed the style-editor-ui branch 2 times, most recently from 83f4ab9 to 246c751 Compare December 6, 2025 18:08
@0tkl
Copy link
Contributor Author

0tkl commented Dec 13, 2025

The code is ready for review, although the commit message appears verbose and warrants discussion. Perhaps a more concise explanation of the rationale behind this change could be provided.

Previously, the Outline/Shadow spinboxes and the Border Style combobox
shared a fixed sizer proportion of 1. Because the combobox has a much
wider natural size than the spinboxes, the wxBoxSizer forced the
spinboxes to stretch excessively to match the combobox's width.

This changes `add_with_label` to use the control's natural width
(`GetBestSize().GetWidth()`) as its sizer proportion instead of a
fixed value.

By weighting the proportion based on width, the layout achieves two
goals:
1. At the default dialog size, the spinboxes are not forced to expand
   to match the wider combobox, allowing them to retain their compact,
   natural size.
2. When the dialog is resized horizontally, all controls expand
   together, distributing the extra space relative to their original
   widths, rather than the combobox remaining fixed or the spinboxes
   growing disproportionately.
@arch1t3cht
Copy link
Member

I actually prefer the suggestion you had before, i.e. just giving the combobox a proportion of 0 and making only the two spin controls expand with the dialog. That helps visually separate the combobox from the spin controls a bit, and is more likely to be what the user wants.

And yes, I'm not really a fan of overly verbose LLM-generated commit messages. Commit messages should explain why a change was made (like e.g. what bugs it fixes) and why a specific solution was chosen over other options, but they don't need to explain the code itself - if the code needs to be explained, that should happen in comments.

@0tkl
Copy link
Contributor Author

0tkl commented Dec 14, 2025

The appearance at various proportions are presented hereby to shed the burden of commit messages by referencing this comment.

Default Stretched
(Before our PR) spinCtrl and non-editable comboBox proportion = 1 1a 1b
spinCtrl and non-editable comboBox proportion = 0 2a 2b
spinCtrl proportion = 1, non-editable comboBox proportion = 0 3b
spinCtrl proportion = 0, non-editable comboBox proportion = 1 4a 4b
spinCtrl and non-editable comboBox proportion = bestWidth 5b
Border style comboBox proportion = 0, anything else proportion = 1 6b

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants