Skip to content

refactor: Host provides editor settings #114

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

Merged
merged 16 commits into from
Apr 24, 2025

Conversation

dcalhoun
Copy link
Member

@dcalhoun dcalhoun commented Apr 5, 2025

Related:

What?

Fetch and cache editor settings before providing them to the editor.

Why?

It allows improving offline support and performance. Ref CMM-200. Fix #105.

How?

Remove editor settings fetch, and replace it with an editor configuration option.

Testing Instructions

See testing instructions for sibling PRs:

Accessibility Testing Instructions

N/A, no user-facing changes.

Screenshots or screencast

N/A, no user-facing changes.

dcalhoun added 2 commits April 4, 2025 20:15
To improve stability, the remote, site-specific editor un-registers
block types that are not on the allow list.
This is largely in place for development experience purposes, not user
facing. It allows the editor WebView to reload and retains its current
GBKit global.
@dcalhoun dcalhoun changed the title feat: Host provides editor settings refactor: Host provides editor settings Apr 5, 2025
@dcalhoun dcalhoun changed the base branch from trunk to feat/block-types-allow-list April 9, 2025 14:47
Avoid exceptions invoking array methods on `undefined`.
@dcalhoun dcalhoun force-pushed the feat/host-provides-editor-settings branch from 9092f64 to cf5773f Compare April 9, 2025 19:57
dcalhoun added 11 commits April 11, 2025 15:30
This reverts commit 38fe2c7.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
This reverts commit 2e1f1464ac65899dc3a5b23af12b59978a34dee9.
To allow the host app to manage editor start sequence, we do not start
within `viewDidLoad`.
Hoist editor settings fetching and configuration to the host app to
allow more control of the editor initialization flow.
Avoid difficulties typing complex JSON structures.
Remove unnecessary or unused code.
Base automatically changed from feat/block-types-allow-list to trunk April 11, 2025 19:38
@dcalhoun dcalhoun force-pushed the feat/host-provides-editor-settings branch from cf5773f to 170d682 Compare April 11, 2025 19:53
const observer = new IntersectionObserver( ( entries ) => {
entries.forEach( ( entry ) => {
if ( entry.isIntersecting ) {
editorLoaded();
Copy link
Member Author

Choose a reason for hiding this comment

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

The editor loaded event now await for the actual visibility of the editor. This ensures the UI is in-place before we visibly present the editor, avoiding UI layout jumps.

Comment on lines +73 to +76
if isWarmupMode {
setUpEditor()
loadEditor()
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Loading the editor is postponed until the host app expressly invokes the methods, allowing the host app to fetch the editor settings.

@dcalhoun dcalhoun marked this pull request as ready for review April 12, 2025 01:14
@dcalhoun dcalhoun requested review from jkmassel and nbradbury April 12, 2025 01:15
@nbradbury nbradbury self-assigned this Apr 15, 2025
Copy link

@nbradbury nbradbury left a comment

Choose a reason for hiding this comment

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

@dcalhoun I tested this with the Android PR and it all looks good. I'll approve, but perhaps you want to hold off merging until someone from the iOS side takes a look? :shipit:

@jkmassel jkmassel mentioned this pull request Apr 16, 2025
@dcalhoun dcalhoun merged commit fa72e63 into trunk Apr 24, 2025
8 of 9 checks passed
@dcalhoun dcalhoun deleted the feat/host-provides-editor-settings branch April 24, 2025 19:52
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.

Host app provides editor settings to improve offline support
3 participants