When navigating to a new Users URL, visible state does not follow
Description
CSP Request Details
CSP Rejection Details
Potential Workaround
Checklist
hideTestRail: Results
Activity
Mike Taylor August 7, 2017 at 3:35 PM
My thoughts are that component state should only be used for things so ephemeral that we don't expect them to survive a trip to another app and back. The search settings should be stored in a local resource.
I don't disagree with that. The issue here is that we have three kinds of state – not just as an artifact of implementation because we really do want three different kinds of behaviour.
1. React component state is entirely transitory.
2. Redux state (including local resources) survives a visit to another module.
3. State in the URL also survives such trips and further is bookmarkable.
From a UI perspective, I feel that much state properly belongs in the third category – as for example the query, filters, sort-order, selected records (all included at present) and whether we're editing or adding a new records (which is being added in other issues).
Charlotte Whitt August 3, 2017 at 9:07 PM
Manually test in FOLIO test http://folio-testing.aws.indexdata.com/, version @folio/users 2.8.100433, using Chrome.
When navigating to a new users URL, visible state does follow.
All looks good. I'll close the issue as done.
Jason Skomorowski August 2, 2017 at 5:31 PM
My thoughts are that component state should only be used for things so ephemeral that we don't expect them to survive a trip to another app and back. The search settings should be stored in a local resource. I really should have added some watchers on .
(The URL should probably be not managed directly and instead automatically reflect a piece of the Redux store too but that's a bigger project.)
Mike Taylor July 27, 2017 at 3:05 PM
is right, of course, that render
is exactly the wrong place to set state.
What I really mean is that we want to set the state from the URL whenever the URL changes. That is probably nicely captured by one of the numerous React lifecycle methods, but I'm not sure which one. We can probably determine is experimentally if necessary.
What's nice about this is that we can take small steps towards the solution we might decide we like best. First, what we have now – explicit `setState`. Then, generating the state to set from the URL, which will give us solution 2. Then, doing so from the appropriate lifecycle method and removing the other `setState` calls, giving us solution 3.
Zak Burke July 26, 2017 at 4:37 PM
I agree with that while this fix is elegant, it addresses a system-wide problem in a one-off manner. But, unless I'm just getting tripped up on the wording, we can't have "the render method always set the state" because we can't call setState
from within render
; render
is called in response to setState
. So I actually lean toward #2, a generic sync-the-state-and-url function that maybe gets called in componentWillReceiveProps
.
As noted in UIU-155, when we navigate to a new URL (
this.props.history.push(path)
, the visible UI state (search term, filters, highlighted sort criterion) are not changed to match what is specified in the new URL. That results in a screen whose UI state does not match the list of records that is displayed.