Improvement to Notes Panel Reply Open/Closed states

[pdftron]

Improvement to Notes Panel Reply Open/Closed states

[justinjung04]

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

[justinjung04]

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.

[ZhijieZhang]

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:

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]

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.