When navigating to a new Users URL, visible state does not follow

Description

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.

CSP Request Details

None

CSP Rejection Details

None

Potential Workaround

None

Checklist

hide

TestRail: Results

Activity

Show:

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.

Done

Details

Assignee

Reporter

Labels

Priority

Fix versions

TestRail: Cases

Open TestRail: Cases

TestRail: Runs

Open TestRail: Runs
Created July 26, 2017 at 10:52 AM
Updated September 7, 2017 at 6:27 PM
Resolved August 3, 2017 at 9:07 PM
TestRail: Cases
TestRail: Runs