Skip to content

More monaco fixes#656

Merged
david-poindexter merged 2 commits intoDNNCommunity:release/0.16.0from
valadas:monaco-fixes
Dec 5, 2022
Merged

More monaco fixes#656
david-poindexter merged 2 commits intoDNNCommunity:release/0.16.0from
valadas:monaco-fixes

Conversation

@valadas
Copy link
Copy Markdown
Member

@valadas valadas commented Dec 5, 2022

  1. In a previous attempt, we were distributing the monaco CSS file as an asset but in DNN context we had no great way to know in which path those files would actually live as we are producing components and not a website. So I reverted to using an scss import as that part was working right, it's only the font that was not working

  2. In order to fix the font I discovered that chrome has a bug with fonts defined inside shadow-root and the workaround is to use scripts to import the font on the document head. I learned that one can use import.meta.url in the context of es-modules to get the URL to the module and not the page or otherwise.

  3. For assets paths, since we have no clue where the path would be for a consumer, I learned that one can use import.meta.url inside an es-module to get the actual path where the module loaded from. We can now use this to reference the assets path correctly no matter where they get distributed by consumers.

  4. I also learned that since Stencil supports workers out of the box, it is better to use getWorker rather than getWorkerUrl for performance and maybe we also avoid some path issues this way as Stencil will know what to bundle how.

  5. I changed some monaco default options that I did not know existed. The most notable one being ariaContainerElement, unless this is set, the default is to inject the aria-text directly at the document end. I am now setting it to a local hidden div for screen-readers only and it no longer shows under the editor.

@valadas valadas added the bug Something isn't working label Dec 5, 2022
@valadas valadas added this to the 0.16.0 milestone Dec 5, 2022
Copy link
Copy Markdown
Contributor

@david-poindexter david-poindexter left a comment

Choose a reason for hiding this comment

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

Fantastic @valadas - only one minor commit I'll make to remove a double space.

link.rel = "stylesheet";
document.head.appendChild(link);
}
let path = import.meta.url.substring(0, import.meta.url.lastIndexOf('/'));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is really cool - I had no clue about this!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah, learned that too digging around...

@david-poindexter david-poindexter merged commit 70f4362 into DNNCommunity:release/0.16.0 Dec 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants