Skip to content
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

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

Open
chrisdburr opened this issue Jun 19, 2024 · 14 comments
Open

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

chrisdburr opened this issue Jun 19, 2024 · 14 comments
Assignees
Labels
blocked! Indicates an issue cannot be progressed bug Something isn't working

Comments

@chrisdburr
Copy link
Collaborator

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.

issue.mp4

Desired Behaviour

Expanded/collapsed states should persist.

@chrisdburr chrisdburr added the bug Something isn't working label Jun 19, 2024
@RichGriff
Copy link
Collaborator

@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
Copy link
Collaborator Author

@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.

issue.mp4

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
Copy link
Collaborator

@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
Copy link
Collaborator

RichGriff commented Jun 20, 2024

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
Copy link
Collaborator Author

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
Copy link
Collaborator

Notes:
@RichGriff to look at the following:

  • Prevent the page reload when creating, updating and deleting elements
  • Need to update the assurance case in state with the new changes (crud)
  • Where hidden value is tracked - changing case re-converts the case into new nodes & edges

@RichGriff
Copy link
Collaborator

RichGriff commented Jun 26, 2024

Progress update

  • ✅ Prevent the page reload when creating, updating and deleting elements
  • ✅ Need to update the assurance case in state with the new changes (crud)
  • Where hidden value is tracked - changing case re-converts the case into new nodes & edges

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

Screen.Recording.2024-06-26.at.19.15.57.mov

@chrisdburr
Copy link
Collaborator Author

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
Copy link
Collaborator

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:

  • Add / Update / Removing doesnt cause already collapsed elements to expand
  • Moving an element, will adopt what its new siblings are set to - if they are hidden the new element will be too
Screen.Recording.2024-07-01.at.15.40.40.mov

@RichGriff
Copy link
Collaborator

Evidence element bug
Screenshot 2024-07-01 at 15 56 21

@RichGriff
Copy link
Collaborator

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

@RichGriff
Copy link
Collaborator

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

@RichGriff
Copy link
Collaborator

@chrisdburr I have managed to retain the hidden value when adding a new link (this was in fact missing but had implemented the logic for when moving elements)

@RichGriff
Copy link
Collaborator

RichGriff commented Jul 24, 2024

Working branch on this issue:
https://github.com/alan-turing-institute/AssurancePlatform/tree/new-element-callopsed-nodes

I can't seem to get this toggle functionality to play ball - it works but the issue is when the child elements are already visible (after adding an child element, when they were hidden) its causing a double click on the toggle button but only the initial time.

Putting this on hold to pick up new features.

@chrisdburr chrisdburr removed workshop-2024 improved navigation Sprint 1: Improved navigation of cases labels Aug 13, 2024
@chrisdburr chrisdburr added the blocked! Indicates an issue cannot be progressed label Nov 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked! Indicates an issue cannot be progressed bug Something isn't working
Development

No branches or pull requests

3 participants