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

Create HTML templates for modals #1425

Merged
merged 10 commits into from
Oct 5, 2024
Merged

Conversation

khaitruong922
Copy link

@khaitruong922 khaitruong922 commented Sep 17, 2024

Fixes #1419

Add loading template modals for ModalController from a list of template names.

Currently having 2 templates (which should have better naming)

  • welcome-modals: Modals shared between welcome and settings page.
  • settings-modals: Modals in settings page only

If this is too vague then we can refactor to multiple templates where each contains a group of modals for a specific setting like: profile-modals, custom-css-modals, anki-cards-modals, etc. then each page will load the required templates.

@Casheeew Casheeew added the kind/meta The issue or PR is meta label Sep 17, 2024
Copy link
Collaborator

@jamesmaa jamesmaa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a good step in the right direction. Is there a way to bundle the template html with its corresponding CSS as well?

ext/templates-modals.html Outdated Show resolved Hide resolved
@khaitruong922 khaitruong922 marked this pull request as ready for review September 21, 2024 15:10
@khaitruong922 khaitruong922 requested a review from a team as a code owner September 21, 2024 15:10
@khaitruong922
Copy link
Author

This seems like a good step in the right direction. Is there a way to bundle the template html with its corresponding CSS as well?

Unlike HTML, the CSS for modal settings.css is not duplicated so it is not violating DRY.

Figuring out all the CSS used for modals may be out of scope of this PR. It requires extracting modals CSS into 2 files shared-modals.css & settings-modals.css and load them in respective page.

@jamesmaa jamesmaa added this pull request to the merge queue Oct 5, 2024
Merged via the queue into yomidevs:master with commit 0d1b36e Oct 5, 2024
12 checks passed
austinyu12 pushed a commit to austinyu12/yomitan that referenced this pull request Oct 22, 2024
* reuse modals

* refactor

* only load templates when needed

* lint

* shared-modals

* lint

* type

* lint

* lint
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/meta The issue or PR is meta
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor modals into separate file
3 participants