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

Issue/1916 fix align issues #1938

Merged
merged 2 commits into from
Feb 20, 2020
Merged

Conversation

SergioEstevao
Copy link
Contributor

Fixes #1916

This PR fixes the set alignment property on RCTAzteView to avoid conflict condition with the alignment property on the base TextView Class.

To test:

  • Open the demo app
  • Add a paragraph block
  • Write some text
  • Set the alignment to Right or Center
  • Check if the alignment is set
  • Tap Enter
  • Check that the alignment stays set on the previous block
  • Switch to HTML
  • Check that the HTML has the alignment attributes
  • Go back to visual and check if the alignment is applied
  • Now tap Enter in the middle of the text of the block
  • Check that both block keep the alignment.

PR submission checklist:

  • I have considered adding unit tests where possible.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

Override set instead of didSet to avoid cross changes of calling
super.set
@SergioEstevao SergioEstevao added the [Type] Bug Something isn't working label Feb 20, 2020
@SergioEstevao SergioEstevao added this to the 1.23 milestone Feb 20, 2020
@SergioEstevao SergioEstevao changed the base branch from develop to release/1.23.0 February 20, 2020 12:31
@mchowning mchowning mentioned this pull request Feb 20, 2020
2 tasks
Copy link
Contributor

@mchowning mchowning left a comment

Choose a reason for hiding this comment

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

Working well! :shipit:

Why do you think didSet was causing the problem? I'm just curious.

@SergioEstevao
Copy link
Contributor Author

@mchowning I didn't fully tracked down the issue, but it looked like when we were using didSet we were calling super.setAlignment and that was causing the value being set on the base UITextView and then I think the didSet on TextView didn't track down properly the oldValue difference.

In either case, I don't think we were doing the correct step of calling a super.setTextAlignment on a didSet.

@SergioEstevao SergioEstevao merged commit ffda54d into release/1.23.0 Feb 20, 2020
@SergioEstevao SergioEstevao deleted the issue/1916_fix_align_issues branch February 20, 2020 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[GlobalStep] iOS – Alignment is not displayed after pressing "return"
2 participants