refactor(gui): drop the macOS spacer column from the dataview lists - #851
Closed
got3nks wants to merge 1 commit into
Closed
refactor(gui): drop the macOS spacer column from the dataview lists#851got3nks wants to merge 1 commit into
got3nks wants to merge 1 commit into
Conversation
Every ported list appended a 1px trailing spacer on macOS. The reasoning was that macOS hands leftover width to the last *resizable* column and collapses it to nothing once the columns are wider than the control, so a throwaway column should absorb that instead of one the user cares about. CGenericClientListCtrl (amule-project#850) shipped without a spacer and does not collapse, which is what prompted checking the rest. The spacer dates from the first base commit, when every list was still constructed with the legacy wxLC_REPORT|wxSUNKEN_BORDER flags -- and wxLC_REPORT is the same bit as wxDV_VARIABLE_LINE_HEIGHT, so those lists were silently running with variable line height. Those flags came off in 71bdb1a and amule-project#849, and with them gone a spacer-less build shows no collapse. So the spacer appears to have been treating a symptom of the flags rather than a macOS sizing rule. Removing it takes the whole mechanism with it: the six call sites, AppendSpacerColumn(), m_hasSpacer, RealColumnCount()'s adjustment and five COLUMN_*_SPACER ids plus CSearchListModel::COL_SPACER, which each model had to answer for. RealColumnCount() stays as the accessor -- now simply GetColumnCount() -- rather than inlining it across its call sites, which is a separate change.
Author
|
Closing: the premise does not hold. Tested a spacer-less build on macOS and the collapse still reproduces -- the last column of a list still disappears once the columns are wider than the control, so the spacer is not a leftover workaround for the legacy The file-details list looked immune only because its two columns (370 + 70) do not overflow its dialog at default widths. Columns are user-resizable, so that is a property of the defaults rather than of the list. The real gap is the opposite one: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Every ported list appended a 1px trailing spacer on macOS. The reasoning was that macOS hands leftover width to the last resizable column and collapses it to nothing once the columns are wider than the control, so a throwaway column should absorb that instead of one the user cares about.
Why it looks unnecessary now
CGenericClientListCtrl(#850) shipped without a spacer and doesn't collapse, which prompted checking the rest.The spacer dates from the first base commit, when every list was still constructed with the legacy
wxLC_REPORT|wxSUNKEN_BORDERflags — andwxLC_REPORTis the same bit aswxDV_VARIABLE_LINE_HEIGHT, so those lists were silently running with variable line height. Those flags came off in71bdb1aand #849, and with them gone a spacer-less build shows no collapse.So the spacer appears to have been treating a symptom of the flags rather than a macOS sizing rule.
What goes
The six call sites,
AppendSpacerColumn(),m_hasSpacer,RealColumnCount()'s adjustment, and fiveCOLUMN_*_SPACERids plusCSearchListModel::COL_SPACER— each of which every model had to answer for.RealColumnCount()stays as the accessor, now simplyGetColumnCount(). Inlining it across its ~18 call sites is a separate change.Testing
macOS Debug,
amule+amulegui, 34/34 tests, clang-format 18 and Tier-2 clang-tidy clean on the diff with 0 compiler errors.Visual check on macOS across the ported lists. The specific thing to watch when reviewing: narrow a list until its columns are wider than the control and confirm the last column doesn't collapse — that is the behaviour the spacer existed to prevent. Sources and Peers are the panes most often narrow; Search additionally syncs columns across tabs.
The spacer was
#ifdef __WXOSX__, so GTK and MSW never had one and removal is a no-op there.