[FIX] Admin panel option is hidden after login #2867
Conversation
kresnaputra
added
🚨 needs changes
🛠 fix
👀 needs review
and removed
🚨 needs changes
labels
diegolmello
left a comment
diegolmello
left a comment
There was a problem hiding this comment.
Left a few comments here and added a new task on ClickUp as blocking.
The issue is really on permissions fetch.
| if (nextState.isAdmin !== isAdmin) { | ||
| return true; | ||
| } | ||
| if (nextProps.state !== state && isSetAdminNeedCall === true) { |
There was a problem hiding this comment.
As you can see at the beginning of the block, state comes from react-navigation (it's track current state of the navigation) and it's an object.
nextProps.state !== state is an object comparison, so it'll never be equal.
| } | ||
|
|
||
| componentDidUpdate() { | ||
| this.setIsAdmin(); |
There was a problem hiding this comment.
Not a good practice to call something on componentDidUpdate without testing in what cases it should run.
componentDidUpdate runs every time the component updates: when state changes, when props change or when the component is forced updated.
Even if shouldComponentUpdate returns false, this.setIsAdmin is being called.
Add a console.log at the start of setIsAdmin and you'll see how many times it's called.
| const isAdmin = permissionsFiltered.reduce((result, permission) => ( | ||
| result || permission.roles.some(r => roles.indexOf(r) !== -1)), | ||
| const isAdmin = permissionsFiltered.reduce((result, permission) => { | ||
| this.setState({ isSetAdminNeedCall: false }); |
There was a problem hiding this comment.
Not a good practice to change the state inside of an iteration.
It's going to set the state for every permissions on the list.
Even if sCU prevents the re-render to happen, you're calling unnecessary resources.
|
Closed in favour of #2914 |
Proposed changes
Fixing admin panel option hiding after login by adding componentDidUpdate function
Issue(s)
How to test or reproduce
Clickup task
Screenshots
Types of changes
Checklist
Further comments