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

Unify initialization procedure of dialogs #1773

Open
jotaen4tinypilot opened this issue Apr 1, 2024 · 0 comments
Open

Unify initialization procedure of dialogs #1773

jotaen4tinypilot opened this issue Apr 1, 2024 · 0 comments
Labels
enhancement New feature or request medium small

Comments

@jotaen4tinypilot
Copy link
Contributor

jotaen4tinypilot commented Apr 1, 2024

Originated in #1770.

The initialization process is basically the same in all our dialogs:

  1. We trigger one operation to initialize the dialog.
    • We often (but don’t always) have a method called .initialize() for this, which is implemented in the respective dialog. In some cases, the method is called differently, but that can be seen as accidental naming inconsistency.
  2. We call .show()

It makes sense for these two things to occur in the stated order, to avoid temporary view state artifacts to flicker when the user opens the dialog.

Considering the amount of dialogs we have, and that they all essentially work in the same way, we could consolidate the flow, and encode a unified initialization process in the code.

Potential Solution

We use an event-based flow:

  • show() could dispatch two custom events into the dialog component:
    • dialog-requested: the dialog may listen to that event, to carry out it’s general init procedure (if applicable)
    • dialog-shown: the dialog may listen to that event and do final adjustments, e.g. for setting focus (if applicable)

I believe the work items for that would be these:

  • Dispatch dialog-requested and dialog-shown events from <overlay-panel> component
    • We may want to declare these events in events.js
  • Set up respective event listeners in all dialogs (if applicable), and remove the initialize() et al calls from app.js
    • Attaching the dialog-shown listener in <paste-dialog> would resolve this issue.
  • Document initialization flow, either in <overlay-panel> component, or in events.js
  • Take care of edge cases (though after briefly scanning the code, there only appears to be one):
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request medium small
Projects
None yet
Development

No branches or pull requests

1 participant