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

Move virtual keyboard outside iframe #204

Merged
merged 12 commits into from
Jul 2, 2024

Conversation

dqnykamp
Copy link
Member

@dqnykamp dqnykamp commented Jun 30, 2024

This PR moves the virtual keyboard outside the iframe if DoenetViewer or DoenetEditor are access via doenetml-iframe. This allows the keyboard to placed at the bottom of the browser window regardless of the size of the iframe.

@dqnykamp
Copy link
Member Author

Got the keyboard to work for the case when we have the iframe. The keyboard no longer works if don't have an iframe. Presumably, we need to have doenetml-iframe send a parameter to doenetml so that we can change behavior based on the context.

@siefkenj
Copy link
Contributor

I think something in the global doenetConfig should be set if we're in an iframe. The RecoilVirtualKeyboard could listen for that and behave normally if it's not in an iframe.

@dqnykamp
Copy link
Member Author

dqnykamp commented Jul 1, 2024

We don't need to know if we are in any iframe; we need to know specifically if we are in an iframe created by the doenetml-iframe package (or, I suppose, any other iframe that adds an <OutsideIframeKeyboard /> outside the iframe). I added a keyboardIsOutsideIframe prop to DoenetViewer and DoenetEditor that is set by doenetml-iframe to make the keyboard look for the outside one.

@siefkenj
Copy link
Contributor

siefkenj commented Jul 1, 2024

We don't need to know if we are in any iframe; we need to know specifically if we are in an iframe created by the doenetml-iframe package (or, I suppose, any other iframe that adds an <OutsideIframeKeyboard /> outside the iframe). I added a keyboardIsOutsideIframe prop to DoenetViewer and DoenetEditor that is set by doenetml-iframe to make the keyboard look for the outside one.

The idea would be that the global config would be set by doenetml-iframe, so a new prop wouldn't be needed.

@dqnykamp
Copy link
Member Author

dqnykamp commented Jul 1, 2024

I'm not sure how to make doenetml-iframe change a global config. Feel free to change the mechanism if that is preferred.

@@ -21,6 +21,7 @@ document.addEventListener("DOMContentLoaded", () => {
undefined,
{
...doenetEditorProps,
keyboardIsOutsideIframe: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't really about whether the keyboard is outside the iframe, as whether the built-in keyboard should be suppressed. Maybe externalVirtualKeyboardProvided?

Copy link
Member Author

Choose a reason for hiding this comment

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

True, though we do then assume that messages come in from window.parent. Not sure if that exists other than with an iframe. I'm happy to rename the prop, though.

packages/doenetml-iframe/src/index.tsx Outdated Show resolved Hide resolved
count: number;
keyboardDomNode: HTMLElement | null;
keyboardReactRoot: Root | null;
callbacks: OnClick[];
};
Copy link
Contributor

Choose a reason for hiding this comment

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

This would look better defined as a type. They you can do

const virtualKeyboardState: VirtualKeyboardState = (window as any).virtualKeyboardState || { ... }

Copy link
Member Author

Choose a reason for hiding this comment

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

done, though now I set window.virtualKeyboardState every time since I don't have an if statement

@dqnykamp dqnykamp merged commit 555d584 into Doenet:main Jul 2, 2024
3 checks passed
@dqnykamp dqnykamp deleted the keyboard-outside-iframe branch July 2, 2024 18:04
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