-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
feat: allow overriding comment icons #7937
feat: allow overriding comment icons #7937
Conversation
return ( | ||
isIcon(obj) && | ||
(obj as any)['setText'] !== undefined && | ||
(obj as any)['getText'] !== undefined && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Forgot to check for setBubbleSize
and getBubbleSize
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess I thought this was intentional because if we're checking from Block
then it could be headless so they don't need to care about bubble size necessarily. But I guess if they only care about headless they prooobably don't have a need to override the class to begin with?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Headless ones also need to have these methods so that the size properly gets round-tripped through serialization.
I also didn't think they were necessary, and I actually left those methods out originally. I only added them to the interface when I ran into the compiler errors :P Which is why I forgot to add them to the type guard.
The basics
The details
Resolves
Fixes #7910
Fixes #7911
Proposed Changes
Allows comment icons to be overriden, and constructed from
Block.prototype.setCommentText
.Reason for Changes
This will allow external partners to create their own comment icons (e.g. that use the new
CommentView
, while still using the built-in functionality likesetCommentText
, context menu options, etc.Test Coverage
Added tests that
setCommentText
properly constructs classes from the registry.Documentation
Yesh, we'll need to document the fact that you can do this somewhere.
Additional Information
N/A