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

Editor: Eliminate or refactor store notices constants #11202

Open
aduth opened this issue Oct 29, 2018 · 2 comments
Open

Editor: Eliminate or refactor store notices constants #11202

aduth opened this issue Oct 29, 2018 · 2 comments
Labels
Framework Issues related to broader framework topics, especially as it relates to javascript Needs Dev Ready for, and needs developer efforts [Package] Editor /packages/editor [Type] Code Quality Issues or PRs that relate to code quality

Comments

@aduth
Copy link
Member

aduth commented Oct 29, 2018

Previously: #9617 (comment)

We now inconsistently use constants for referencing notices by ID in the editor module. The primary need for them is in dismissal upon the start of a new save (e.g. autosave notice, previous save success/error notice).

We should consider one of:

  • Reintroduce consistency, either eliminating all instances of constant references to notice IDs in favor of strings (yes, magic strings), or reintroducing (introducing?) constant reference use.
  • Implement a better system to express and track notices which ought to be dismissed in response to particular actions, like "on next save". See Packages: Add notices package #9617 (comment)

In #9617, it was chosen to not use the constant to avoid a dependency between the component and the store, though this is loosely held.

@aduth aduth added Framework Issues related to broader framework topics, especially as it relates to javascript [Type] Code Quality Issues or PRs that relate to code quality [Package] Editor /packages/editor labels Oct 29, 2018
@paaljoachim
Copy link
Contributor

Should this issue be closed?

@gziolo

@gziolo
Copy link
Member

gziolo commented Jan 13, 2021

There are still 2 places where we use constants so I would say this issue is close to ready but not done yet.

@gziolo gziolo added the Needs Dev Ready for, and needs developer efforts label Jan 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Framework Issues related to broader framework topics, especially as it relates to javascript Needs Dev Ready for, and needs developer efforts [Package] Editor /packages/editor [Type] Code Quality Issues or PRs that relate to code quality
Projects
None yet
Development

No branches or pull requests

3 participants