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

Make CDN optional & minor code improvements #1

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

ma4nn
Copy link

@ma4nn ma4nn commented Nov 21, 2023

Great idea to integrate the Monaco Editor in Magento/OpenMage 😍

This PR suggests several improvements:

  • add composer.json file
  • make frontend dependencies explicit via package.json
  • replaced mm_monacoeditor/system_config_source_cms_pages with model from core
  • fixed small issue with legacy admin theme
  • added PHP typehints
  • deferred JavaScripts

Hope you like it. Looking forward to your feedback!

…in core), minor code style fixes, fixed editor overlay for default/default admin theme, add tailwind css only if active, remove dependency from cdn
@empiricompany
Copy link
Owner

empiricompany commented Nov 22, 2023

Wow, there are a lot of changes! Thanks for your contribution, but maybe it's better to split them into single PRs. Please give me time to review and test all.

@empiricompany empiricompany self-assigned this Nov 22, 2023
@@ -0,0 +1,3 @@
.hor-scroll {
overflow: initial;
}
Copy link
Owner

Choose a reason for hiding this comment

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

What problem does this solve? I have tried it with the default theme and I did not encounter any issues even without this CSS.

Copy link
Author

Choose a reason for hiding this comment

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

Without this fix it may look like this:
Bildschirmfoto 2023-11-23 um 11 14 15

@empiricompany
Copy link
Owner

By deleting workers in js/monacoeditor folder, Tailwind IntelliSense does not work.
tailwindcss-intellisense-not-work

…ripts as this causes problems with tailwind workers
@ma4nn
Copy link
Author

ma4nn commented Nov 23, 2023

By deleting workers in js/monacoeditor folder, Tailwind IntelliSense does not work.

Yeah you're right, this was not very clever🤦‍♂️ I re-added the deleted files and removed JavaScript deferring because that also caused issues with Tailwind Autocompletion.

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.

2 participants