Skip to content

Fix copy/paste of attributed string from app without support for dark/light mode - #1237

Merged
SergioEstevao merged 7 commits into
developfrom
issue/fix_color_attributes
Nov 21, 2019
Merged

Fix copy/paste of attributed string from app without support for dark/light mode#1237
SergioEstevao merged 7 commits into
developfrom
issue/fix_color_attributes

Conversation

@SergioEstevao

Copy link
Copy Markdown
Contributor

Fixes #wordpress-mobile/WordPress-iOS#12688

This PR implements three fixes to sort issues around text color:

  • When copy/paste attributed string it makes sure that text color is set to default color if no color is set
  • It also make sure that when all text is deleted the typing attributes are reset to default attributes that include the default color
  • Make sure that the defaultColor is used for default attributes where converting from HTML mode.

To test:

  • Copy paste text from other apps. Examples: Notes app, Tumblr in post edit mode, inside different sections of Aztec
  • Open the demo app, put the cursor over the text with the red color, switch to HTML mode, and switch back to visual. Check that text is displayed in the correct color.
  • Open the demo app, select all the text and delete it. Start to type check that the text shows with the default attributes and not with the attributes of the last selection.

@maxme do you want these fixes to land on release 13.7 of WPiOS (currently frozen) or the next one 13.8?

@etoledom etoledom left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Works great! 🎉

Tested on iOS 13 and 12 ✅

I left a couple of questions as code comments, but ✅ since the issue is solved.
Great work!

Comment thread Aztec/Classes/TextKit/TextView.swift Outdated
Comment thread Aztec/Classes/TextKit/TextView.swift Outdated
@maxme

maxme commented Nov 21, 2019

Copy link
Copy Markdown
Contributor

@maxme do you want these fixes to land on release 13.7 of WPiOS (currently frozen) or the next one 13.8?

IMO, It got the criterias to get merged in the current frozen version:

  • It's a bug fix.
  • You added tests.
  • It's not too big and not impacting the rest of the app too much (less prone to create a blocker).

So let's merge it in 13.7.

@frosty frosty left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good, just two small comments – but otherwise :shipit:

Thanks for fixing this!

Comment thread Aztec/Classes/TextKit/TextView.swift Outdated
Comment thread Aztec/Classes/TextKit/TextViewPasteboardDelegate.swift
@SergioEstevao

SergioEstevao commented Nov 21, 2019 via email

Copy link
Copy Markdown
Contributor Author

@SergioEstevao
SergioEstevao merged commit 9e83d77 into develop Nov 21, 2019
@SergioEstevao
SergioEstevao deleted the issue/fix_color_attributes branch November 21, 2019 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants