-
Notifications
You must be signed in to change notification settings - Fork 2
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
Separate InlineToolbar UI and business logic #91
Conversation
⏭️ No files to mutate for |
Coverage report for
|
St.❔ |
Category | Percentage | Covered / Total |
---|---|---|---|
🟢 | Statements | 100% | 0/0 |
🟢 | Branches | 100% | 0/0 |
🟢 | Functions | 100% | 0/0 |
🟢 | Lines | 100% | 0/0 |
Test suite run success
1 tests passing in 1 suite.
Report generated by 🧪jest coverage report action from fe1b213
Coverage report for
|
St.❔ |
Category | Percentage | Covered / Total |
---|---|---|---|
🟢 | Statements | 96.74% | 800/827 |
🟢 | Branches | 98.17% | 214/218 |
🟢 | Functions | 88.07% | 192/218 |
🟢 | Lines | 96.62% | 772/799 |
Test suite run success
404 tests passing in 24 suites.
Report generated by 🧪jest coverage report action from fe1b213
⏭️ No files to mutate for |
Coverage report for
|
St.❔ |
Category | Percentage | Covered / Total |
---|---|---|---|
🟢 | Statements | 86.11% | 62/72 |
🟡 | Branches | 62.5% | 15/24 |
🟢 | Functions | 100% | 10/10 |
🟢 | Lines | 86.11% | 62/72 |
Test suite run success
6 tests passing in 1 suite.
Report generated by 🧪jest coverage report action from fe1b213
* - provides methods to work with selection | ||
*/ | ||
@Service() |
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 think, that comment, about who would call selection api will be helpfull
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.
Anybody who needs it
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.
Anybody who needs it, that's public API
packages/core/src/api/index.ts
Outdated
export class EditorAPI { | ||
@Inject() | ||
public blocks!: BlocksAPI; | ||
|
||
@Inject() | ||
public selection!: SelectionAPI; | ||
} |
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.
leave comments about injected parameters please
import type { Index, InlineToolName } from '@editorjs/model'; | ||
|
||
/** | ||
* Payload of BlockAddedCoreEvent custom event |
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.
seems irrelevant
*/ | ||
#eventBus: EventBus; | ||
|
||
#inlineTools: Map<InlineToolName, InlineTool> = new Map(); |
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.
comment required
/** | ||
* Remove link input, when data formed and trigger callback | ||
*/ | ||
linkInput.remove(); | ||
|
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 this change is needed?
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.
Just to keep link input there, doesn't matter atm as UX will be updated anyway
No description provided.