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

Improvement to Notes Panel Reply Open/Closed states #78

Closed
henry-chris opened this issue Dec 10, 2018 · 4 comments
Closed

Improvement to Notes Panel Reply Open/Closed states #78

henry-chris opened this issue Dec 10, 2018 · 4 comments
Assignees
Labels
enhancement New feature or request need more info Needs more information to proceed

Comments

@henry-chris
Copy link

henry-chris commented Dec 10, 2018

Reply text box should remain open if you click off or onto the the PDF as long as you do not select another annotation.

If there are a lot of annotations and you click off it makes every annotation look the same (closed state), so it can be hard to tell where you were just typing. Also, since you have many annotations, any usage of the mousewheel to scroll will scroll the Notes Panel instead of the document, so you must click out to scroll (which makes you lose your annotation in the list).

Could be something for individual users to sort out, but we thought it was more of an overall improvement to the UI.

The text area / text box in question:

image

@justinjung04
Copy link
Contributor

Thanks for the suggestion! We will put this into the backlog and resolve it when we work on usability improvements.

@justinjung04 justinjung04 self-assigned this Dec 17, 2018
@justinjung04 justinjung04 added the enhancement New feature or request label Dec 17, 2018
@justinjung04
Copy link
Contributor

justinjung04 commented Jun 14, 2019

Just wanted to follow up, we tried testing on our online demo and we could not reproduce this issue:

any usage of the mousewheel to scroll will scroll the Notes Panel instead of the document

What we observed is that you can scroll through the document with your reply open & focused if you hover over the document and scroll (without clicking). We also talked to our design team, and they were worried about breaking the consistency between selected annotations and expanded notes.

@justinjung04 justinjung04 added the need more info Needs more information to proceed label Jun 14, 2019
@ZhijieZhang
Copy link
Contributor

ZhijieZhang commented Aug 22, 2019

Hey Chris,

any usage of the mousewheel to scroll will scroll the Notes Panel instead of the document

I'm still having trouble reproducing this issue. When you have time, can you double check if this issue still occurs? I'm attaching a gif below about what I see:

notes-panel-scrolling

Regarding the open/closed state for the Note component. Unfortunately after another internal discussion we decided not to do it for now.

Currently the NotesPanel is listening to the annotationSelected event to determine if a Note is open.

https://github.com/PDFTron/webviewer-ui/blob/48d2db3bbaa574d897ec8b3e248554a5334f7a01/src/components/NotesPanel/NotesPanel.js#L82-L95

And by default if a user clicks somewhere in the document container, all of the annotations will be deselected. This means this event will be triggered again and corresponding notes will be closed.

To make the notes remain open, we could make the code look like this:

 useEffect(() => { 
   const onAnnotationSelected = (e, annotations, action) => {
     if (action === 'selected') {
       const ids = {}; 
  
       core.getSelectedAnnotations().forEach(annot => { 
         ids[annot.Id] = true; 
       }); 
       setSelectedNoteIds(ids); 
     }
   }; 
  
   core.addEventListener('annotationSelected', onAnnotationSelected); 
   return () => 
     core.removeEventListener('annotationSelected', onAnnotationSelected); 
 }, []); 

However this will make the note not closable by clicking on itself.

https://github.com/PDFTron/webviewer-ui/blob/48d2db3bbaa574d897ec8b3e248554a5334f7a01/src/components/Note/Note.js#L37-L49

I've thought about letting the Note component control the open/closed state itself instead of letting the NotesPanel component do this. But with a recent virtualization change to the notes panel, this is not doable anymore... When there're more than 300 notes in the panel, only those are visible will be rendered. This means that if the Note component is keeping the open/close state, the state will be lost when it's unmounted.

One last option that might work is to pass setSelectedNoteIds down as a prop to each Note so that Note can control the open/close state. But this feels kind of weird to me.

Let me know how important is this improvement for you. If it's important I can dig into this more and figure something out.

@henry-chris
Copy link
Author

Hi. Sorry, I had forgotten all about some of these issues.

So, I know I tested this on your online demo because we totally overwrite the notes panel and put it back on the right side with a different UI/UX. So my picture is from your demo.

However, that was about 9 months ago so many things could have changed since then. Testing today's demo, it seems the issue no longer exists. As you've demonstrated focus is correctly given to the document if the mouse is in the document and is given to the NotesPanel if it's there. So that can be closed.

The open/closed state is honestly not a big problem. It was something our QA brought up and we thought it could be an improvement. Thanks for giving it some thought and anything you decide will be just fine.

This issue can be closed as far as I'm concerned.

Thanks again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request need more info Needs more information to proceed
Projects
None yet
Development

No branches or pull requests

3 participants