FlowFuse / FlowFuse/flowfuse

Dialogs Phase 1: dialogs should have their state handled by their parent components

Open
#4,028 6 comments 0 reactions 0 assignees View on GitHub
area:frontend needs-triage size:M story
Dominant language
JavaScript
Stars
400
Forks
89
Avg merge
1d 21h
Merged PRs (30d)
146

Description

### Epic

#4000

### Description

We need to change the way we use dialogs across the front end of the application.

We're currently using the HTMLDialogElement show method which causes concurrency issues when rendering. It is a best practice when using vue to use only vue when manipulating DOM elements.

A side effect of this is that all dialogs are present at all times in pages where they are used (but hidden with display:none) and:
- unnecessarily loading the DOM with unused elements
- creating difficulties when testing with cypress because we have to pick the exact dialog we're referring to
- adding state management problems in which certain methods/watchers are being called unnecessarily within these dialogs even though they are not in use.

General anti pattern
Calling child component methods from the parent is similar to calling parent/root component methods from the child component in that it tightly couples the components together and is considered an anti-pattern. The recommended way is for two way parent-child is props passed down to child components by the parent and events emitted by child components and caught by parent components.

One example I can give, which was my own doing (I now realize), is for the blueprint tiles. Each blueprint tile has the AssetDetailDialog inside.

This implies that, with our current configuration, we have a hidden AssetDetailDialog present for each blueprint tile present on the page.

At the moment there are no real implications on this other than the fact that we have multiple hidden modals. But if we were passing props, or the dialog was using computed store props or state keys and was reacting via watchers/computed props, it would unnecessarily add computational overhead to page renders.

What I've done in https://github.com/FlowFuse/flowfuse/pull/3994/files#diff-0d94818a781a7387946662af02b3ed79ce7d1999841488af4d4985121cb3565b addresses this issue by conditionally rendering the modal in the parent. It's a small change, with the implication that the modal's state is determined externally from the dialog which somewhat breaks encapsulation but it resolves the aforementioned side effects because the dialog is not mounted until it is decided by the parent.

### Which customers would this be available to

Everyone - CE/Starter/Team/Enterprise

### Acceptance Criteria

- [ ] Dialogs should have their state dictated by their parent components and not be render/present in the DOM when not in use.
- [ ] Dialogs should not manage their visibility via HTMLDialogElement show methods

check BlueprintSelectorDialog.vue for reference

Phase two should should implement a single global dialog which is present in the front end, it's state being linked to a ux store which can swap out components. It should be globally accessible and have the possibility to be interacted with across all components via the uxStore

### Have you provided an initial effort estimate for this issue?

I have provided an initial effort estimate

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.