-
Notifications
You must be signed in to change notification settings - Fork 7
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
INT-2756: add statusbar
attribute
#56
Conversation
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.
Does this need a changelog item?
@spocke changelog entry added, have a look |
@@ -48,6 +48,7 @@ const parseNumberOrString = (value: string) => /^\d+$/.test(value) ? Number.pars | |||
|
|||
const configAttributes: Record<string, (v: string) => unknown> = { | |||
setup: parseGlobal, // function | |||
statusbar: parseBooleanOrString, // boolean |
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.
Should this be // boolean or string
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 thought the same, but it is a boolean: https://www.tiny.cloud/docs/tinymce/latest/statusbar-configuration-options/#statusbar
I think we both confused with toolbar
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.
Hmm, then parseBooleanOrString
should be parseBoolean
?
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 used that because is what is used for other fields like powerpaste_allow_local_images
and images_upload_credentials
, maybe I'm wrong but the attribute will be always a string so even true
and false
will be "true"
and "false"
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.
Right since they are all strings it needs to check for specific values or fallback to the string value. Makes sense now. I really should have spent time to look thought the webcomponent plugin this sprint but all other stuff came in between.
982da10
to
c9666e4
Compare
INT-2756
issue: #24