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

added new scroll to demo logic #13

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

Conversation

VKR981
Copy link

@VKR981 VKR981 commented Aug 15, 2023

New scroll logic

Editor.webm

}
}

window.addEventListener('DOMNodeInserted',onDomChange)
Copy link
Member

Choose a reason for hiding this comment

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

it's better to bind this event inside the onMounted hook to prevent SSR issues

Copy link
Author

Choose a reason for hiding this comment

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

I changed it, I placed mobile focus part inside setTimeout because it solves to problems

  1. Virtual keyboard interrupts scroll
  2. Editor is focused before it's initialised

@VKR981 VKR981 marked this pull request as draft August 16, 2023 06:56
@VKR981 VKR981 marked this pull request as ready for review August 18, 2023 02:16
Comment on lines 85 to 105
(document.querySelector('.fake-input') as HTMLElement).focus();

const block: HTMLElement | null = document.querySelector('.ce-paragraph');
const block: HTMLElement | null = document.querySelector('.ce-paragraph');

if (block !== null){
if (block !== null){

block.focus()
}
block.focus()
}
},1000)
}

/**
* Send analytics event
*/
$track(AnalyticEvent.PlayWithDemoClicked)
}


/**
* Scroll the target element to 33% from top of the screen
*/
Copy link
Member

Choose a reason for hiding this comment

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

You have problems with identations

@@ -77,22 +77,47 @@ function playDemoClicked(){
* contenteditable element which is not presents on a page in a moment of button click
*
* So we use fake invisible input to focus it first, then change focus to our editor
*
* setTimeout is used because the virtual keyboard interrupts scroll
Copy link
Member

Choose a reason for hiding this comment

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

This problem and solution is unclear to me. Do you mean mobile devices?

For now, it breaks the focus of editor on mobile devices (tested in Safari), which is working fine in main. Also, scroll works fine in main on mobile safari

Copy link
Author

@VKR981 VKR981 Aug 22, 2023

Choose a reason for hiding this comment

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

Are you sure because I tested it on Safari (IOS Simulator), it was working fine, and yes the focus and scroll breaks on Chrome (Android) main.

Copy link
Author

@VKR981 VKR981 Aug 22, 2023

Choose a reason for hiding this comment

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

Another solution is to put
`const block: HTMLElement | null = document.querySelector('.ce-paragraph');
if (block !== null){

    block.focus()
  }`

inside onReady callback which is woking on Chrome (Android) and Safari(IOS Simulator).

Copy link
Author

Choose a reason for hiding this comment

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

I have kept the changes to a minimum this PR only addresses the scroll issue on web tested on Chrome (Desktop) and Safari (Desktop)

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.

3 participants