alan-turing-institute / AssurancePlatform

Project to facilitate creation of Assurance Cases
MIT License
17 stars 6 forks source link

[BUG] Branches do not remain collapsed when adding a new element #492

Open chrisdburr opened 2 weeks ago

chrisdburr commented 2 weeks ago

Issue

If a user has collapsed a branch, and then goes on to add a new element to a separate branch, when the case refreshes all branches are expanded.

Note in this video how S3 is collapsed, and then a new element is added to a child of S4. When the case is refreshed, S3 is expanded.

https://github.com/alan-turing-institute/AssurancePlatform/assets/63010234/f244f82b-ea8d-432c-804a-b5bf1b3c5129

Desired Behaviour

Expanded/collapsed states should persist.

RichGriff commented 2 weeks ago

@chrisdburr this is the expected behaviour as we reload the chart after adding a new element. Which pulls the new updated assurance case and re-creates the nodes for that case, updating the assurance case in the state. Therefore, removing any elements that had been toggled to hide.

chrisdburr commented 2 weeks ago

@RichGriff, I appreciate it may be the expected behaviour, but it's not desirable as a user.

Here's another example of how this is frustrating. In this example, I add a new element to the right-hand side of the case and when the case reloads I am moved to the left-hand side of the case.

https://github.com/alan-turing-institute/AssurancePlatform/assets/63010234/1254f72f-7212-43f7-9514-b1464f46e85c

Constantly moving around to get back to where you were is not good UX. If this needs to be revised into a broader issue, and not a bug, that's fine. But we need to change this behaviour to improve the user experience.

RichGriff commented 2 weeks ago

@chrisdburr The only thing we can do here then is not reload the screen which is what we had before we put in place the identifier stuff.

RichGriff commented 2 weeks ago

Would need to store the hidden value for each node.

Myself and @cptanalatriste - have checked the feasibility of this and have identified about 1 week of work. @chrisdburr to discuss next sprint planing.

chrisdburr commented 2 weeks ago

Thanks, both. It was flagged as an annoyance by multiple users in our workshops, so I think it's probably worth the effort. Let's discuss next week.

RichGriff commented 1 week ago

Notes: @RichGriff to look at the following:

RichGriff commented 1 week ago

Progress update

The last part here is going to be difficult as we currently re-render the nodes when the assurance case in state changes

https://github.com/alan-turing-institute/AssurancePlatform/assets/28445922/6e34c6c2-cccb-41e3-934a-4a0f1715e2c9

chrisdburr commented 1 week ago

Big improvement with just these first fixes, @RichGriff.

I don't understand the final issue with the hidden value, but appreciate the status update.

RichGriff commented 4 days ago

So this one has been a little pain but i think i have nailed the last point - retaining to the hidden state.

The below video should show the following:

https://github.com/alan-turing-institute/AssurancePlatform/assets/28445922/7a19e475-1df6-402b-9355-2e35462c5187

RichGriff commented 4 days ago

Evidence element bug

Screenshot 2024-07-01 at 15 56 21
RichGriff commented 4 days ago

Incorrectly setting hidden value on evidence. Correction was to look at at parent hidden if there are no siblings.

RichGriff commented 4 days ago

This is now in staging @chrisdburr can you please review.