feat(highlight): Change cursor in Highlight Text mode#544
feat(highlight): Change cursor in Highlight Text mode#544mergify[bot] merged 3 commits intobox:masterfrom
Conversation
mingzexiao6
force-pushed
the
add-highlight-cursor
branch
from
4760fbb to
ab9ec85
Compare
mingzexiao6
force-pushed
the
add-highlight-cursor
branch
from
ab9ec85 to
f10c5be
Compare
|
|
||
| // Construct a layer element where we can inject a root React component | ||
| const rootLayerEl = documentEl.createElement('div'); | ||
| rootLayerEl.classList.add('ba-Layer'); |
There was a problem hiding this comment.
Do we have any concerns about rendering this layer even though the feature is not yet available?
There was a problem hiding this comment.
If the toolbar highlight button is not clicked, we actually only render a <div /> with absolute positioning and pointer-event: none. So I don't have any concerns.
There was a problem hiding this comment.
Gotcha. I think @ConradJChan had some concerns about rendering these components unconditionally, but I don't recall why. It should be okay for now, but we'll need to keep this in mind if we add anything else outside of the "creator" components.
| jest.mock('../../common/withProviders'); | ||
| jest.mock('../HighlightAnnotations'); | ||
|
|
||
| describe('HighlightContainer', () => { |
There was a problem hiding this comment.
Should we follow the path format here? I'd also be game for nuking the path entirely and just using the component name. Maintaining the path here seems tedious.
There was a problem hiding this comment.
Will nuke the path entirely in the followup pr.
jstoffan
left a comment
jstoffan
left a comment
There was a problem hiding this comment.
LGTM, just make super sure there are no regressions for region annotations.
2 participants
Uh oh!
There was an error while loading. Please reload this page.