-
Notifications
You must be signed in to change notification settings - Fork 129
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
Fix History Back, Push Bugs #1447
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.
This is great, thanks for refactoring everything so neatly, love all that red. Just left a "tiny little" comment.
@@ -25,10 +25,11 @@ const StyledInput = styled.input` | |||
outline: none; | |||
} | |||
` | |||
// create a tiny little history closure |
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.
Is this comment referring to this component or just a leftover note to self?
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.
A left over note to self!
Fixes #1331
There were 3 implementations of a History class in the app:
This PR creates a single
app/core/models/history.ts
which is used in all the places the above 3 classes were used.The bug was ins the "push" method of both
LogDetailHistory.ts
andInputHistory.ts
. It did not properly "splice" the new record at the correct position, then delete all following entries. It simply "pushed" a new entry onto the array. Thebrim/entries.ts
implementation did have it correct.In the first commit I create a failing test that reproduces the bug.
To Do: