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

Logging support for viewer app #2001

Merged
merged 6 commits into from
Aug 26, 2020
Merged

Logging support for viewer app #2001

merged 6 commits into from
Aug 26, 2020

Conversation

RFSH
Copy link
Member

@RFSH RFSH commented Aug 25, 2020

This PR,

  • Adds proper log support to the viewer app (Logging in viewer app #1910). For the complete list of actions please refer to this google sheet.
  • Add extra log info to distinguish between view button and the rest links (log eye v.s. link when direct to a record page #1917)
  • Fix a bug in the viewer app which would cause users to be stuck in the edit form (Open edit form for an annotation, don't change anything and click on save.)
  • Unhighlight annotations when switching to edit mode.
  • Removed the unnecessary usage of slide_id in the code.
  • Fixed a small bug in the logic of disabling the "show/hide annotation" button. It wasn't properly checking the value that osd viewer is returning.

@RFSH RFSH self-assigned this Aug 25, 2020
@RFSH RFSH changed the title Viewer logging Logging support for viewer app Aug 25, 2020
RFSH added 2 commits August 25, 2020 11:30
this commit will also fix the scrollToView function in viewer.
It was scrolling the whole page instead of the scrollable left panel.
@RFSH RFSH requested a review from jrchudy August 25, 2020 21:48
@jrchudy
Copy link
Member

jrchudy commented Aug 26, 2020

The only thing I had a comment about was adding the logging functions to the annotation service. I think it makes sense to do it there but was wondering why in annotation service instead of the existing log service?

@RFSH
Copy link
Member Author

RFSH commented Aug 26, 2020

@jrchudy I've done the same in table.js and faceting.js as well. I also did the same in recordedit in this PR (in recordCreate.js).

These functions are more specific to the ones in log service. They're mainly some wrappers that use the contextualized data in each place (for example in table.js it's using the logStack attached to vm or in annotation service I'm using the logStackNode that is attached to each annotation model) and call the general functions in log service. We could eventually move all these functions to log service and pass the contextualized data as well if we find a more general pattern in them.

@RFSH RFSH merged commit 657e7c7 into master Aug 26, 2020
@RFSH RFSH deleted the viewer-logging branch August 26, 2020 18:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants