Skip to content
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

feat(highlight): Change cursor when hovering over text #547

Merged
merged 3 commits into from
Jul 30, 2020

Conversation

mxiao6
Copy link
Contributor

@mxiao6 mxiao6 commented Jul 29, 2020

Need to merge box/box-content-preview#1239 first

@mxiao6 mxiao6 requested a review from a team as a code owner July 29, 2020 00:08
Copy link
Collaborator

@jstoffan jstoffan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should reconsider our approach to keep things decoupled and reusable.

src/highlight/HighlightCreator.scss Outdated Show resolved Hide resolved
src/document/DocumentAnnotator.scss Outdated Show resolved Hide resolved
src/highlight/cursor.scss Outdated Show resolved Hide resolved
src/document/DocumentAnnotator.ts Outdated Show resolved Hide resolved
src/document/DocumentAnnotator.ts Outdated Show resolved Hide resolved
src/highlight/HighlightAnnotations.scss Show resolved Hide resolved
@mxiao6 mxiao6 force-pushed the cursor-hover-text branch from 4d7ce98 to 6932b9f Compare July 30, 2020 00:39
src/common/BaseAnnotator.ts Outdated Show resolved Hide resolved
src/common/BaseAnnotator.ts Outdated Show resolved Hide resolved
src/document/DocumentAnnotator.scss Outdated Show resolved Hide resolved
Copy link
Collaborator

@jstoffan jstoffan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just needs tests.

@jstoffan jstoffan self-requested a review July 30, 2020 01:02
@mxiao6 mxiao6 marked this pull request as ready for review July 30, 2020 17:08
@mxiao6 mxiao6 force-pushed the cursor-hover-text branch from d6c0280 to 0b445d0 Compare July 30, 2020 17:18
@@ -57,6 +65,18 @@ export default class DocumentAnnotator extends BaseAnnotator {
return this.annotatedEl ? Array.from(this.annotatedEl.querySelectorAll('.page')) : [];
}

handleChangeMode = ({ mode }: { mode: Mode }): void => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

handleModeChange?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually followed the naming convention in BaseAnnotator like handleSetVisible and handleSetActive

@mergify mergify bot merged commit e18c6b3 into box:master Jul 30, 2020
@mxiao6 mxiao6 deleted the cursor-hover-text branch August 13, 2020 00:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants