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.

Sign up for GitHub

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

Jump to bottom

feat(scroll): emit 'annotations_initialized' when annotations first initializes. #515

Merged
merged 8 commits into from
Jun 16, 2020
Merged

feat(scroll): emit 'annotations_initialized' when annotations first initializes. #515

merged 8 commits into from
Jun 16, 2020

Conversation

mickr
Copy link
Contributor

@mickr mickr commented Jun 11, 2020

box/box-content-preview#1224 depends on this change

@mickr mickr requested a review from a team as a code owner June 11, 2020 17:37
@mickr
feat(scroll): add method to return annotation by Id
503063e 
* accommodate changes in content-preview to send ready event with annotations once annotations are loaded.
@mickr mickr changed the title feat(scroll): add method to return annotation by Id feat(scroll): add ready event for content-preview Jun 12, 2020
@jstoffan
Copy link
Collaborator

Should we emit annotations_fetch_success when the fetch call succeeds, instead? This will only fire once per lifecycle of the annotation.

@mickr
Copy link
Contributor Author

mickr commented Jun 12, 2020

Should we emit annotations_fetch_success when the fetch call succeeds, instead? This will only fire once per lifecycle of the annotation.

The document isn't ready to scroll when the fetch call completes as there are no annotated elements and the page elements are not loaded yet.

jstoffan
jstoffan reviewed Jun 15, 2020
View reviewed changes
src/common/BaseAnnotator.ts Outdated Show resolved Hide resolved
src/common/BaseAnnotator.ts Outdated Show resolved Hide resolved
mickr added 2 commits June 15, 2020 16:43
@mickr
feat(scroll): add method to return annotation by Id
d3069d0 
* Refactor to emit 'annotations_initialized' using eventing middleware
@mickr
feat(scroll): add method to return annotation by Id
31563fc 
* Remove un-used class variable
jstoffan
jstoffan reviewed Jun 15, 2020
View reviewed changes
src/store/annotations/selectors.ts Outdated Show resolved Hide resolved
src/common/BaseAnnotator.ts Outdated Show resolved Hide resolved
@jstoffan
Copy link
Collaborator

Can we update the PR title?

@mickr mickr changed the title feat(scroll): add ready event for content-preview feat(scroll): emit 'annotations_initialized' when annotations first initializes. Jun 16, 2020
@mergify mergify bot merged commit 6021585 into box:master Jun 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ConradJChan ConradJChan ConradJChan left review comments

@jstoffan jstoffan jstoffan approved these changes

Assignees
No one assigned
Labels
ready-to-merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mickr @jstoffan @ConradJChan