-
Notifications
You must be signed in to change notification settings - Fork 41
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
ui: Reset state for workflow item history when changing tabs #318
Conversation
224e03f
to
1fb6c4f
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.
I believe to folder structure can be improved. I would move WorkflowItemDetails into the WorklowItem folder
@@ -35,7 +35,8 @@ export default function reducer(state = initialState, action) { | |||
currentHistoryPage: action.currentHistoryPage, | |||
isHistoryLoading: false | |||
}); | |||
|
|||
case CLOSE_WORKFLOWITEM_HISTORY: | |||
return initialState; | |||
case CLOSE_WORKFLOWITEM_DETAILS: |
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.
Use fallthrough and handle logout action
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
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.
also added the LOGOUT
action here
@@ -144,6 +145,12 @@ function WorkflowDetails({ | |||
}, | |||
[showWorkflowDetails] | |||
); | |||
useEffect( |
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 effect is called every time, even if it is not necessary. I believe this hook has to move into the component itself.
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.
changed implementation of <WorkflowitemHistoryTab>
to class based and used componentWillUnmount
to reset state
2fa66be
to
af0daf7
Compare
import { fetchNextWorkflowitemHistoryPage, resetWorkflowitemHistory } from "./actions"; | ||
|
||
class WorkflowitemHistoryTab extends React.Component { | ||
constructor(props) { |
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.
Remove, not needed
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
When a user views the workflow item details and clicks on history and then back to another tab, the history state is now reset, so the calculation of offset and limit are correct. Add e2e test that makes sure this does not happen again. Refactor e2e tests, so that 'data-test' is used instead of 'id'. Closes #317 Rework based on review - Moved WorkflowitemHistoryTab into Workflows - Rewrote WorkflowitemHistory to class based component - Removed effect hook for resetting history state and moved it to ComponentWillUnmount - Fixed e2e-tests - Added LOGOUT action to WorkflowitemHistory reducer
73a7d79
to
d7ef6e5
Compare
Changes
When a user views the workflow item details and clicks on history and
then back to another tab, the history state is now reset, so the
calculation of offset and limit are correct.
Add e2e test that makes sure this does not happen again.
Refactor e2e tests, so that 'data-test' is used instead of 'id'.
Related issues
Closes #317
How to test
When viewing a workflow item's history, there should be no issues.