netcreateorg / netcreate-itest

Developing the 2.0 version of NetCreate
https://github.com/netcreateorg/netcreate-2018
Other
0 stars 0 forks source link

Comment edit lock #175

Closed benloh closed 2 months ago

benloh commented 2 months ago

While a comment is being edited, we need to catch events to prevent the comment from inadvertently being closed:

To Test

  1. Open a node that already has a comment
  2. Click the comment button to open the window (but don't edit or hit reply)
  3. Try:
    • open NodeTable or EdgeTable and view or edit another node/edge -- you should be able to do it
    • in the selected node, click edit -- you should be able to edit the node
    • click the comment button -- you should be able to close the comment window
    • in the graph, click to select a node -- you should be able to select it (and close the prior comment)
  4. Now add a new comment
    • open NodeTable or EdgeTable -- the view and edit buttons in the table should be hidden
    • in the selected node, the "Edit" button should be disabled so you can't click it
    • click the comment button -- nothing should happen -- you can't click the comment button to close the comment
    • in the graph, click to select a node -- nothing should happen -- you can't select another node
  5. Save the comment -- everything should work again.
  6. Add a reply -- everything should be locked again.

We probably want to add some messaging so that the caught clicks don't just quietly fail. e.g. add an alert message informing the user to "Please finish editing the comment before selecting another comment" or something like that.

benloh commented 2 months ago

Consider adding a generalized click manager #174

jdanish commented 2 months ago
Screenshot 2024-04-25 at 11 59 21 AM

Tried running npm ci and same error ... ?

benloh commented 2 months ago

Sorry 'bout that. Hopefully this is fixed?

jdanish commented 2 months ago

Yes it runs now!

jdanish commented 2 months ago

Minor bug - if editing a node closes comment window, the most recent comment is not marked as read:

  1. Open comment on node 1
  2. Add a reply
  3. Click save
  4. The comment tab shows this is unread
  5. Close comment window
  6. There is no-longer an unread comment

BUT ...

  1. Open comment on node 1
  2. Add a reply
  3. Click save
  4. The comment tab shows this is unread
  5. Click edit on node
  6. Comment window closes, shows unread

Otherwise that seems fine ...

jdanish commented 2 months ago

Note: it used to be that if I opened a comment and selected a new node, the old comment stayed open. Now when I click to a new node, it switches to the comments from that new node. I think that might feel confusing.

jdanish commented 2 months ago
Screenshot 2024-04-25 at 12 51 58 PM

Also, I think this was already happening, but the comment icon keeps the red outline after being cleared unless you reload the window.

benloh commented 2 months ago

Minor bug - if editing a node closes comment window, the most recent comment is not marked as read

This is currently by design: we don't mark a comment read unless you explicitly click on the comment "X" or "Close" comment. Otherwise, any time you open a comment window and open a new node everything would be marked read even if you didn't scroll to actually see the comment (e.g. if the comment is long and has scrolled off the screen). I suppose the question is whether we want to err on the side of marking more comments read or on making the act of marking a comment read more explicit.

Note: it used to be that if I opened a comment and selected a new node, the old comment stayed open. Now when I click to a new node, it switches to the comments from that new node. I think that might feel confusing.

I think this has been the behavior for quite a while now. The problem is that we want to associate the comment window with the comment button. As soon as you select a different node, the comment button is closed. If we kept the old comment window, we would have an open comment window but no sense of where that comment window came from. Selecting a node from the node table does let you keep multiple comment windows open, but that's because that comment button is still visible in the table.

We probably want to add an indicator that shows which comment button is open and associated with the opened comment.

Also, I think this was already happening, but the comment icon keeps the red outline after being cleared unless you reload the window.

Hmm...I'll need to revisit this. Especially if we want to use the stroke to indicate which window is open.


The comment locking and marking read/unread is fairly complicated, so I think there are still some instances where things are not being properly unlocked. I still need to do a lot of QA.

jdanish commented 2 months ago

This is currently by design: we don't mark a comment read unless you explicitly click on the comment "X" or "Close" comment. Otherwise, any time you open a comment window and open a new node everything

OK that makes sense.

I think this has been the behavior for quite a while now. The problem is that we want to associate the comment window with the comment button. As soon as you select a different node, the comment button is ...

Ahh, somehow I missed it and I liked the old version. I somehow had multiple comments open, but ... if it is a huge ask I can verify people's feelings on it before you put in the work.

benloh commented 2 months ago

I think this has been the behavior for quite a while now. The problem is that we want to associate the comment window with the comment button. As soon as you select a different node, the comment button is ...

Ahh, somehow I missed it and I liked the old version. I somehow had multiple comments open, but ... if it is a huge ask I can verify people's feelings on it before you put in the work.

Coordinating open comment windows across a node, an edge, the nodetable and the edgetable was not easy. I can see why it might be handy to have multiple comment windows open, but I think it's problematic that a comment window remains open after the node itself has been closed. It would mean, for instance, you could not easily click through multiple nodes to quickly view comments. It'd take five clicks to review two nodes:

  1. click node to open
  2. click comment button to open comment
  3. click node to open second node
  4. click comment button on second node to open second comment
  5. click close on first comment to close the first comment.

I suppose the alternate approach would be to always close the comment window on selecting a new node, but then you'd lose the ability to have mutliple comment windows open in the first place.

jdanish commented 2 months ago

Hm. OK, given the context let's go with changing what is in the window for now (so don't change it). The majority of the time, if I am not "testing" I would close the comment myself so it's not really an issue. Let's run with that.

benloh commented 2 months ago

Joshua commented: "Would it be easy to add a title on the comments window? So in that bar it might say "Comments on Node: Fish"? That'll help make it obvious where you are if you click around and ease some of my discomfort with that functionality. Then I'd say let's leave it for now. Also, confirming the recent fix to the icon (no more red outline when there shouldn't be)." in #178