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
Some recent repositories settings improvements #10903
Some recent repositories settings improvements #10903
Conversation
As I said, I really want to see a beginning of improvements on this form included in the next release. |
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.
+0
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.
appears sensible to me
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.
+0
Changes seem fine to me but not run and no deep review.
(and pmiossec, I am not innocent of delaying releases myself, but I want the normal update to start so we can get a 4.2 out).
@mstv's comments are still unresolved. |
@pmiossec could you please resolve the outstanding feedback - action or reject - so that we could merge this. This seems to be the last outstanding item tagged for v4.1. Thanks |
when: - destination list was initially empty - changing shortening strategy
I don't see a good reason to display the separator only when a one of the list is sorted.
004a776
to
432e5a7
Compare
Done. |
AllRecentLB.BeginUpdate(); | ||
|
||
SetComboWidth(); | ||
RefreshRepos(); | ||
} | ||
finally | ||
{ |
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.
PinnedLB.BeginUpdate();
AllRecentLB.BeginUpdate();
seems to be not necessary here any longer.
Though it should be added to RefreshRepos()
.
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.
done (With a little bonus: improvement over the bad UX of the "Combobox minimum width" that I had to use for the 1st time to test this piece of code)
@@ -35,6 +37,7 @@ private void LoadSettings() | |||
SetNumericUpDownValue(_NO_TRANSLATE_maxRecentRepositories, AppSettings.MaxPinnedRepositories); | |||
SetNumericUpDownValue(_NO_TRANSLATE_RecentRepositoriesHistorySize, AppSettings.RecentRepositoriesHistorySize); | |||
|
|||
_previousValue = comboMinWidthEdit.Value; |
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.
_previousValue = comboMinWidthEdit.Value; | |
_previousValue = comboMinWidthEdit.Value; | |
return; | |
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.
done.
by jumping directly to values that has an effects i.e 0 to 30 (values between 1 and 30 will have the same effect than 30) Prevent the user to have to click 30 times on a up or down buttons to see an effect.
ff2b1df
to
92a1d0e
Compare
I have selected only the accepted and already reviewed commit of #3769 to see some improvements made for v4.1
Note: Small commits that are easily reviewed one by one.
Configure repositories popup:
Repositories list:
Screenshots
Configuration
Before
After
Menu
Before
After
With separator:
Test methodology
Test environment(s)
Merge strategy
I agree that the maintainer "Merge commit" (better) or "Rebase merge" this PR.
✒️ I contribute this code under The Developer Certificate of Origin.