RichTextAction subscription #129
Conversation
|
@danielsaidi Can you please take a look when you have some free time to ensure we are going correct direction? This is breaking change, changing published properties to internal for them to reflect only and only be used with binding(for)... |
Sources/RichTextKit/Coordinator/RichTextCoordinator+ColorSubscriptions.swift
Outdated
Show resolved
Hide resolved
DominikBucher12
force-pushed
the
improvement/richtextaction
branch
3 times, most recently
from
b4ccd3b
to
b4bd81d
Compare
DominikBucher12
force-pushed
the
improvement/richtextaction
branch
from
7c76788
to
076651f
Compare
| case .setUnderlineColor: .underlineColor | ||
| case .setStrikethroughColor: .strikethroughColor | ||
| case .setStrokeColor: .strokeColor | ||
| case .setColor(_, let richTextColor): |
There was a problem hiding this comment.
@DominikBucher12 I thiiiiink the rich text color has a title that you can return right away.
There was a problem hiding this comment.
It actually does not :( I wouldn't implement it in there IMHO, but the icons can stay.
| case .strikethrough: .strikethroughColor | ||
| case .stroke: .strokeColor | ||
| case .underline: .underlineColor | ||
| case .undefined: .strokeColor |
There was a problem hiding this comment.
Is this correct for the undefined?
There was a problem hiding this comment.
You need to return something and this feels a lot better than exception or cascade of optionals. I would personally focus after 1.0 to get away from undefined...
|
|
||
| /// Whether or not the latest undone change can be redone. | ||
| @Published | ||
| public var canRedoLatestChange = false | ||
| var canRedoLatestChange = false |
There was a problem hiding this comment.
But actually, removing public from these are breaking changes. I think we can do it if we don't release this PR as a 0.9 patch, but as a 1.0 bump.
I also don't understand how other parts of an app would be able to refresh if they can't listen to these published properties?? 🤔
What this PR do:
userActionPublisher!How to test it
TODO