search

alan-turing-institute / AssurancePlatform

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

[BUG] When the goal claim is collapsed, the assurance case seems to disappear #491

Closed chrisdburr closed 1 day ago

chrisdburr commented 2 weeks ago

Issue

When working with large assurance cases, collapsing the goal claim creates a situation where it seems as though the case has disappeared. While it is just the case that the case is off-screen, and clicking the Focus button in the toolbar brings back the case, it is confusing for new users.

https://github.com/alan-turing-institute/AssurancePlatform/assets/63010234/01ada469-3d19-4ec8-bea0-df5540ffdcc3

Furthermore, when the case is then expanded, it is not centred upon the element that has been expanded.

Desirable behaviour

  1. When an assurance case element is expanded/collapsed, the viewport should remain centred upon the expanded/collapsed element.
RichGriff commented 2 weeks ago

@chrisdburr You previously asked me to remove the re-focus element when collapsing nodes as you said it was weird behaviour? As this is the reason why i added it so it would always center for users.

The toggle on goal is only happening for larger charts so will look into why this is happening.

chrisdburr commented 2 weeks ago

@chrisdburr You previously asked me to remove the re-focus element when collapsing nodes as you said it was weird behaviour? As this is the reason why i added it so it would always center for users.

The toggle on goal is only happening for larger charts so will look into why this is happening.

I think there was a miscommunication here, @RichGriff. The desirable behaviour is for the case to remain centred on the specific element that was being edited—not the centre of the case, which will necessarily change as elements are added/deleted.

RichGriff commented 2 weeks ago

@chrisdburr the only thing that is available it the fitView() which will center the case - as it tries its best to fit the chart in the current viewport - I havent come across anything that focuses on the node that has been selected (will investigate).

However, I have been looking at the bug issue, and tried to re-apply some logic to the toggle function. Can you review the video below please and let me know what you think.

https://github.com/alan-turing-institute/AssurancePlatform/assets/28445922/d611528b-ea6b-4e97-ab7c-dd9ec99f6d7e

I have set all nodes to hidden by default apart from the goal, so this then gives the user to start from the goal and navigate through.

RichGriff commented 2 weeks ago

The case doesnt disappear - what happens here is that the gola node is out of view, because the fitview() function isnt being used to bring it back in view when collapse/expanding child nodes. Hence, why it comes back to view when the focus button is selected.

I have reverted the chart to show all nodes by default so we have the same behaviour but set the fitView() to run when using the toggle feature.

RichGriff commented 2 weeks ago

Adding the fitView() function back in can be in staging immediately. However, we can discuss in print planning if you want to go in a different direction @chrisdburr (this will take some time to research)

chrisdburr commented 2 weeks ago

Looking at your video, @RichGriff, this already looks like an improvement. Happy to test when it's available in staging.

cptanalatriste commented 2 weeks ago

There's an approach in staging. Please check if suitable

chrisdburr commented 2 weeks ago

Adding the fitView() function back in can be in staging immediately. However, we can discuss in print planning if you want to go in a different direction @chrisdburr (this will take some time to research)

The approach in staging is improved in one respect (i.e. the case doesn't seem to disappear off screen when the goal claim is collapsed). However, the refresh on collapse/expanding and adding/removing for other elements, especially in large cases, is still creating undesirable behaviours that some of our workshop users suggested were a frustration.

For example, here we add a new element and when the case refreshes the viewport is in a different section:

https://github.com/alan-turing-institute/AssurancePlatform/assets/63010234/37743432-49a0-4e41-95a2-6d81e354c043

And, here, when we collapse/expand a branch, the case moves around to an area that is not the point of focus. Some of workshop users stopped working in the platform because of this behaviour:

https://github.com/alan-turing-institute/AssurancePlatform/assets/63010234/ab8954ec-ad5e-40df-b4ad-a1cc34b558ac

chrisdburr commented 2 weeks ago

Linking to #492, as I expect the solution to this issue will also be relevant to the other bug.

RichGriff commented 1 week ago

There are two different things here:

chrisdburr commented 1 week ago

Can we look at this in our sprint planning on Wednesday please? I will need to see the actual behaviours to determine the right course of action.

RichGriff commented 4 days ago

We have fixed this as part of the collapsing issue as these two are tightly linked.

The fix here was to only focus on goal element when toggling, otherwise dont focus.

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

chrisdburr commented 1 day ago

Thanks. I can confirm that this is behaving appropriately now in the case of collapsing the goal claim. Will need to do more thorough testing for the wider adding/removing and expanding/collapsing of other elements with a good variety of assurance cases (e.g. small/large, simple/complex).