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

Fixed issue with editor window snapping upward on iOS when ... #33

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rahimrahman
Copy link

scrolling to the bottom. See:

#32 #32

Without fix:

editor_snap_at_bottom_ios

With fix:

fix-editor-snap-upward

@rahimrahman
Copy link
Author

@artald what do you think?

@artald
Copy link
Collaborator

artald commented Jun 21, 2017

@rahimrahman thanks for the PR!

I think that it's really great that you found a solution for this, however, I find it a little bit weird that it's solved by doubling the keyboard height.
Do you know why?

@rahimrahman
Copy link
Author

rahimrahman commented Jun 21, 2017

@artald in the example app, I need any value bigger than 90 pixels, because the padding on top of the rich text editor is set to 40 and the rich text toolbar height is 50 which amount to 90. But I've tried with any value from 90 - 999, and it seems to fix the issue.

In reality, you could use

const editorAvailableHeight = -1;

for https://github.com/wix/react-native-zss-rich-text-editor/blob/master/src/RichTextEditor.js#L92 and it seems to have fixed the issue as well.

So we could require user to specify another prop and call it offset (value would be 90 in the example app), or just change the value for editorAvailableHeight line 92 of RichTextEditor.js to -1.

@rahimrahman
Copy link
Author

@artald any thoughts on my explanation ^^?

@gameboyVito
Copy link

A simple solution is setting the footerHeight props, like this:

const {width, height} = Dimensions.get('window');
<RichTextEditor
       ref={(r) => this.richtext = r}
       style={styles.richText}
       footerHeight={height * 0.2}    
       editorInitializedCallback={this.onEditorInitialized}/>

@artald
Copy link
Collaborator

artald commented Jun 27, 2017

Thanks guys.

@rahimrahman your explanation sounds good :) though it still doesn't really tell us why adding an additional offset fixes this issue.

Having an additional prop or a way to configure this would be better since I don't know if anyone will have time to check all the use-cases and make sure it doesn't affect or make things worse in different cases.

@d4vidi what do you think? can someone from the team look into it?

@rahimrahman
Copy link
Author

@artald i thought I had made it obvious that the height of the html editor needed to be smaller by 90 pixels in the example app because the example app didn't take into account the size of the padding on top of the editor (40 px) & and the height of the of the toolbar (50 px). By reducing the editor height value by 90, the editor is not snapping upward anymore.

I still think by setting const editorAvailableHeight = -1 in the package src will solve all use cases (unless somebody can show me other use cases that the above change fails). Now I can't explain why setting editorAvailableHeight = -1 fixes this issue, maybe some other folks could dissect editor.html js function pertaining to how zss.setEditorHeight actually works.

@rahimrahman
Copy link
Author

@artald

simulator screen shot jun 27 2017 5 20 15 am

@gameboyVito
Copy link

Dear all, how to disable the horizontal scroll indicator? Currently, when I focus on the content, the whole page will scroll to the left and this is not a good behavior when I disable the title section.

@artald
Copy link
Collaborator

artald commented Jun 28, 2017

@rahimrahman got it, thanks, it's now clear regarding the padding and toolbar height, but as you said - now it's not so clear why editorAvailableHeight set to -1 solves it as well..

Maybe the safest bet would be to add a prop to opt-in to this fix, otherwise we'll need to verify that this doesn't break anything in some use-case that maybe we didn't think of. Unfortunately, at the moment I don't have the capacity to check it myself.

@d4vidi @ihork @yevhenpavliuk FYI, any insights would be welcome. It would also help if you can test the solution with your case.

fkesheh pushed a commit to trilogy-group/react-native-zss-rich-text-editor that referenced this pull request Dec 12, 2022
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