Fix popup of 'Target technology for cost analysis' preference #62
Conversation
veger
force-pushed
the
fix-targettech-popup
branch
from
5ae2105 to
16d514d
Compare
|
It would make reviewing easier if refactors were commit-separated from the change of logic. I'll add that to the contributor's guide. |
|
Regarding the change itself, it would be awesome to add the docs of at least the parts that we understand because the names of the methods rarely explain what exactly it is about. For instance, if you know what the changed popups are about, then please add a word or two about them as a |
|
In particular, I'd like to see their summaries because that would show what they are about and if the width parameter is fitting there. |
veger
force-pushed
the
fix-targettech-popup
branch
2 times, most recently
from
ae5ff06 to
0b584a3
Compare
|
I split the PR into 2 commits (refactor + fix). I tried to add a summary for the 2 modified functions, but it is quite unclear what the (full) functionality of these (GUI) functions is, as there are so many parameters and nested layers... |
veger
force-pushed
the
fix-targettech-popup
branch
from
0b584a3 to
b69e6fe
Compare
The header is too wide for the default width of 20f. Make the width configurable (as it is used in more places) and increase the width of this particular popup to fit the text/label.
veger
force-pushed
the
fix-targettech-popup
branch
from
b69e6fe to
6c00690
Compare
The header is too long for the default width of 20f. Make the width configurable (as it is used in 3 places) and increase the width of this particular popup.
Also fixed some typos in related code.
Fixes #27