-
Notifications
You must be signed in to change notification settings - Fork 28
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
Add reset number of commits to show for a branch button to experiments table #4355
Conversation
b5d81d4
to
6f8cf27
Compare
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.
Great work!
))} | ||
{commitsButtons.map(commitButton => { | ||
const { key, ...commitButtonProps } = commitButton | ||
return <CommitsButton key={key} {...commitButtonProps} /> |
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.
Why not use the index provided by the map function as the key
value? Then we could remove the key
pair from the commitsButton
arr.
@@ -478,6 +478,10 @@ export class ExperimentsModel extends ModelWithPersistence { | |||
this.persistNbOfCommitsToShow() | |||
} | |||
|
|||
public resetNbfCommitsToShow(branch: string) { | |||
delete this.numberOfCommitsToShow[branch] |
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.
I've looked over the code multiple times but I can't figure out how this change is saved in the state across sessions. The function only touches this.numberOfCommitsToShow
yet if I reload the window, the change is saved so the numberOfCommitsToShow
is being persisted somewhere, right? How is it working though 😅 What am I missing?
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.
The memento could just be holding a reference to the original object. I will make an update.
afc8dd4
to
0c1190c
Compare
… experiments table
0c1190c
to
fed39c4
Compare
Code Climate has analyzed commit fed39c4 and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 100.0% (85% is the threshold). This pull request will bring the total coverage in the repository to 95.2% (0.0% change). View more on Code Climate. |
Related to #4269
Demo
Screen.Recording.2023-07-26.at.2.45.35.pm.mov