-
Notifications
You must be signed in to change notification settings - Fork 33
Conversation
Codecov Report
@@ Coverage Diff @@
## main #62 +/- ##
==========================================
+ Coverage 86.65% 86.67% +0.01%
==========================================
Files 75 75
Lines 667 668 +1
Branches 77 78 +1
==========================================
+ Hits 578 579 +1
Misses 85 85
Partials 4 4
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
simpleSelectors.blockValue, | ||
]); | ||
}); | ||
it('returns true iff unitUrl, blockValue, and editorInitialized are all truthy', () => { |
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.
This was intentional. From math iff = "if and only if"
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.
Or has this behavior changed?
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.
Oh! I thought this was a typo. Let me change back.
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.
more importantly, why has this behavior changed?
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.
This was Ben's change. Without the change, the text editor stays in loading mode (isInitialized remains false). I'll have to look a bit deeper to see what editorInitialized was doing.
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 confused why editorInitialized
is being removed-- I assume its becuase editors are no longer tied to tinymce but confused why this happened in this PR in particular?
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.
Wait I get the editorintialized thing-- we need to remove it as there is no longer a callback which triggers it once tinymce loads in, and it was a bug fix in the first place. This better promotes the abstraction layers we want, so as long as it works, I'm in.
No description provided.