-
Notifications
You must be signed in to change notification settings - Fork 25
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use calculated width for first column of Summary page #167
Use calculated width for first column of Summary page #167
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me
I'll rebase against @have-fun-was-taken maybe we should disable the reviews getting dismissed on new commits? This requires re-reviewing over and over... |
e05e9ea
to
51ea298
Compare
The idea was that it improves the code review by forcing the reviewer to check the final code. The reality is that it became annoying when each rebase must be re-approved. Sure, I'll disable it. |
I understand the initial idea, and I support this completely in order to be sure the changes are noticed and reviewed again. Maybe we can have a 'guideline' to manually ask for an re-approval when we updated the PR by addressing the feedback? |
So it is much more convenient to find the width of a text without actually allocating the rectangle in the GUI.
This prevent unnecessary/reserved width for this colum
51ea298
to
0ac3048
Compare
If we have too many rules, it might feel stifling. |
@have-fun-was-taken can you re-approve this PR, so we can merge it? |
Reading it now |
With this PR the fixed width (introduced by #38) will be no more and the column width is dynamically calculated depending on the names of the production sheets.
This change is part of a larger set of changes that I am planning to commit, but those changes need some additional love... (It works, but I broke some things that should not be broken).
This change is ready and works separately and adds value to YAFC already.
I had to apply one small hack to make this a stand-alone PR, which is making
firstColumnWidth
static. In the upcoming PR it will be a regular field of its class.