-
-
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
Rewrite ckeditor5-cloud-services and ckeditor5-ckbox to TypeScript. #13314
Conversation
|
||
import CKBoxUI from './ckboxui'; | ||
import CKBoxEditing from './ckboxediting'; | ||
import type { TokenUrl } from '@ckeditor/ckeditor5-cloud-services/src/cloudservices'; |
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.
Move above local imports
*/ | ||
async getAvailableCategories( offset = 0 ) { | ||
public async getAvailableCategories( offset: number = 0 ): Promise<Array<{ |
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.
Consider creating a type for Category
/** | ||
* @inheritDoc | ||
*/ | ||
public static get requires(): ContextPluginDependencies { |
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.
Why not just PluginDependencies
?
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.
CloudServices
is a ContextPlugin
, not a Plugin
. Returning PluginDependencies
would make it incompatible with PluginCollection
used in Context
.
* | ||
* Without a properly working token endpoint (token URL) CKEditor plugins will not be able to connect to CKEditor Cloud Services. | ||
*/ | ||
tokenUrl: TokenUrl; |
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.
It's not optional on purpose?
Co-authored-by: Michał Remiszewski <118178939+mremiszewski@users.noreply.github.com>
…o ck/13005-ts-ckbox
Suggested merge commit message (convention)
Other (ckbox): Rewrite
ckeditor5-ckbox
to TypeScript. Closes #13005.Other (cloud-services): Rewrite
ckeditor5-cloud-services
to TypeScript. Closes #13007.Additional information
For example – encountered issues, assumptions you had to make, other affected tickets, etc.