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

Add external libraries development mode. #542

Closed
wants to merge 1 commit into from

Conversation

dclause
Copy link
Contributor

@dclause dclause commented Mar 17, 2023

This PR follows the discussion at #524
The PR is far to be anything but good. It just "POCs" the ideas detailled in the ticket 524.

@falkoschindler
Copy link
Contributor

falkoschindler commented Mar 17, 2023

Yes, using a PR to showcase an idea is a good strategy. 👍
I can see where this is going and I definitely like it. Nevertheless, I'll probably continue without the production flag for now to keep things simple(r) for the upcoming 1.2.0 release. Later in 1.2.1 we should be able apply this PR on top.

Just one thought regarding your current implementation: Is there a reason you moved around the imports? E.g. quasar.css and nicegui.css changed places. And Vue and Quasar JS imports moved down below the #app div. I think we should be careful here because changing the order can lead to quite subtle changes in behavior.

@dclause
Copy link
Contributor Author

dclause commented Mar 17, 2023

External libraries are usually loaded at the bottom of the body (but before local script code) and deferred. I did not go with defer here, but that could be worth a try.

As of the CSS, I grouped them together in the header. My bad though, I made something stupid: the order should be external libraries, external CSS framework, internal overrides. So nicegui.css should be the last on to load.

@falkoschindler falkoschindler linked an issue Apr 14, 2023 that may be closed by this pull request
@falkoschindler falkoschindler removed a link to an issue Apr 14, 2023
@falkoschindler
Copy link
Contributor

In version 1.3.0 the dependency management changed quite a bit, making the proposed changes a bit obsolete. I'll close this PR for now, since it is pretty outdated and stale for months now. But feel free to reopen it or create a new one to continue working on this feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants