-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Store compact view status #3850
Conversation
@@ -33,6 +34,15 @@ export const RecordIndexBoardContainerEffect = ({ | |||
getOnFetchMoreVisibilityChangeState, | |||
} = useRecordBoard(recordBoardId); | |||
|
|||
const { currentViewSelector } = useViewScopedStates({ | |||
viewScopeId: viewBarId, |
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 used that way in other places in the code. Should I do a refacto?
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.
No, we'll take care of that while refactoring the ViewBar later this month
description: 'Describes if the view is in compact mode', | ||
defaultValue: { value: false }, | ||
}) | ||
isCompactView: boolean; |
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.
isCompact?
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.
@Weiko updated
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.
Backend lgtm 😄
@@ -33,6 +34,15 @@ export const RecordIndexBoardContainerEffect = ({ | |||
getOnFetchMoreVisibilityChangeState, | |||
} = useRecordBoard(recordBoardId); | |||
|
|||
const { currentViewSelector } = useViewScopedStates({ | |||
viewScopeId: viewBarId, |
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.
No, we'll take care of that while refactoring the ViewBar later this month
} | ||
// We don't want to re-run this effect when the state changes. | ||
// We only want to make sure that the state reflects the current view. | ||
// eslint-disable-next-line react-hooks/exhaustive-deps |
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.
We should not try to block react re-rendering. It's an effect, it is not a problem :)
Currently, compact view status is only set in the state. This is not stored in DB so it gets reset when the page is refreshed.
This PR stores it as a boolean in the view.
Enregistrement.de.l.ecran.2024-02-06.a.16.24.41.mov