netcreateorg / netcreate-itest

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

Comment UI First Pass #133

Closed benloh closed 3 months ago

benloh commented 5 months ago

Here's a first pass at a UI for commenting.

The main intent right now is to work through what the basic UI might look like.

Please give it a spin and let us know your first impressions. We're still working through issues (see TO DO) but the key functionality is in place.

A walkthrough:

  1. When you first select a node, comments will be displayed immediately. screenshot_1398

  2. Clicking on the comment text field will add a comment and put it in edit mode. screenshot_1399

    Comment author and creation date are displayed, along with the comment type and an input field.

  3. You can select different comment types screenshot_1402

    By default the comment type is a generic comment.

  4. Comment types can have an arbitrary number of fields. screenshot_1403

    Currently if you start with multiple input fields (e.g. a type with three fields), then change type to a type with fewer fields (e.g. two fields), the third extra field will not be shown. But if you switch to another three field type, all three old answers will be shown. This should make it relatively safe to repeatedly change comment types (or recover from selecting the wrong type) without worrying about losing your data.

  5. Comment types have optional help text and feedback text screenshot_1404

    Comment types, help, and feedback are all customizable and will be a part of the project template or perhaps a separate comment template. You can play with it here in app/view/netcreate/comment-mgr.js. See line 42's definition of DB_CommentTypes.

  6. Clicking "Save" will save the comment and return it to View mode. screenshot_1405

    In "View" mode you can see the current comment number.

  7. Hover over the comment to see secondary functions: "EDIT" and "DELETE" screenshot_1411

    We put "EDIT" and "DELETE" in a menu to keep the comments decluttered, and to discourage it's use. This emulates Google Doc's approach.

  8. Hover over the comment to see the "Reply" button screenshot_1408

  9. Click on the "Reply" to add a comment thread. screenshot_1412

  10. You can also reply to a thread. screenshot_1413

    The threading is limited to one level of nesting.

  11. You can also add a new comment to the root thread (start a second conversation) by clicking on the "Click to add a Comment..." text field. screenshot_1414

  12. Click on a different node to start a new separate comment collection.

TO DO

These will be fixed as we continue to work on the PR:

jdanish commented 5 months ago

Awesome!! This is a great start and the templating is particularly satisfying. I'll run past the team, but 3 quick notes:

1: putting focus in the top field would be nice but is minor so let's just flag and leave until much later

2: I like the reply appearing but the screen resizing negate of it is slightly jarring. Minor issue.

3: Most importantly, I think having the comment be an icon on the node that pops open a floating window like in meme still feels ideal to me. I'll ask the team but was there any technical reason not to do it that way? That'll read easier in the tables too I think.

Loving it though!!

benloh commented 5 months ago

2: I like the reply appearing but the screen resizing negate of it is slightly jarring. Minor issue.

Sorry I'm not sure I'm quite following. You mean the "Reply" button shows only on hover, so the comment sizes shift? I was trying to figure out a good workaround for that. I wanted to keep the height as minimal as possible while still making the "Reply" button large and easily clicked.

3: Most importantly, I think having the comment be an icon on the node that pops open a floating window like in meme still feels ideal to me. I'll ask the team but was there any technical reason not to do it that way? That'll read easier in the tables too I think.

For Net.Create, it seemed like minimizing the number of clicks to view a comment would be good. The general idea was to have a global "View Comments" button that would show or hide comments. And then you could click on an node and immediately view the comment -- so any node comment browsing is essentially one click. Otherwise, any review of a comment would require two clicks: one to select a node, a second to open the comment. If we have a floating comment window, that would increase it to three clicks: one to select a node, a second to open the comment, and a third to close the comment so you can select another one.

Things get even worse with Edges, because you have to:

  1. select a node
  2. select the Edges tab
  3. select an edge
  4. click to open the comment
  5. click to close the comment to select another one.

The downside is on a smaller screen, it's harder to see the comment and the node at the same time.

Since we probably do need floating windows for MEME, I can wrap it in a floating window to see what it feels like.

For the table view, are you thinking that all of the comments and replies would sit in a single table row?

jdanish commented 5 months ago

You mean the "Reply" button shows only on hover, so the comment sizes shift?

Yes. I'm not sure what's ideal, so let's ignore for the moment and focus on bigger issues.

I agree with minimizing clicks, but I think we end up with more clicks if you count the scrolling to see the comments below the node definition, and I'm not even sure where the edge one would go. So, if it's not a huge deal to wrap it similar to how we did in MEME to test, that'd be great.

For the table view I was thinking we'd just have the icon as a column and then pop it open. Unless you mean the "comment table" or "comment list" that we've discussed. For the nodes / edge table, see attached.

Screenshot 2024-02-05 at 2 54 40 PM
benloh commented 4 months ago

Second Pass: Floating Comment windows

Updated 2024-02-10

NOTE: Do a npm ci please because there's a new library react-draggable added.

The key change here is moving to a floating comment window. This requires many more clicks to view comments. Edge comments are not yet supported.

CAVEATS

benloh commented 4 months ago

@jdanish a general question to ponder: comments currently have numeric ids, as do nodes and edges, but they are not mutually exclusive. This means we can have three objects with the same number. e.g. node 10 is different from edge 10 is different from comment 10. Is this a problem? Or should each object have a unique number? What would be less confusing?

benloh commented 4 months ago

Third Pass: Add Comment Count and Comment Icon for Comment Button

My concern is that many users are used to icon count badges meaning the number of "unread" comments/messages rather than the total count of comments/messages. (e.g. iOS Messages app, Email app, notifications, etc.). Though I can see why it's helpful to always be able to see the total count of messages. I think we'll just have to try this to see if people grok it.

benloh commented 4 months ago

Fourth Pass: Add Edge Comments

Technical Notes

Nodes and edges use the same sequential ids, but for comment collection references, we add a a "nX" or "eX" prefix to the collection reference. This way comments, nodes, and edges can all simple ids starting at 0 without having to worry about conflicts.

Caveats

benloh commented 4 months ago

Fifth Pass: Comment buttons in tables

screenshot_1434

Caveats

jdanish commented 4 months ago

This generally looks great.

For the caveats - I see how having multiple open comments could be confusing, but it's easy enough to just close one, or close them all and re-open the one you want to work with. Is there something else I didn't manage to trigger? Otherwise ... I am OK with that for now.

benloh commented 4 months ago

Great!

I'd say poke around a little with the comment windows and see if you come up with a situation that needs to be resolved. I've seen a few instances where things felt squirelly, but it's not critical, and mostly comes from abusing comment windows. So if you don't feel like it's an issue perhaps we can punt on it for now.

benloh commented 4 months ago

@jdanish If a user deletes a comment do we just remove it and relink any threads? e.g. if you remove the second of three replies, we just connect the dangling third one to the first? Do we want to leave a "Comment delete" placeholder? Or do we just actually remove it?

jdanish commented 4 months ago

By "Or do we just actually remove it?" do you mean remove everything beneath it?

I'm leaning towards:

I believe one of the spec requirements / requests is that we'd be able to set the level at which people are allowed to delete. In cases where we are worried that kids might delete important conversations and social handling isn't enough we'd restrict deleting to the teacher (I think that's the current MEME model). Otherwise we'll assume decent social behavior.

Compared to say word or google docs, there would be no resolve / un-resolve option so once a comment is gone it is gone though we can find it in the log file for research purposes.

benloh commented 4 months ago

By "Or do we just actually remove it?" do you mean remove everything beneath it?

Yeah, remove the comment and any nested replies that follow.

set the level at which people are allowed to delete

We'll have to revisit this when we get user auth working. Currently a user can only delete their own comment.

benloh commented 4 months ago

BTW there's some threading logic that is currently broken...still working on it. You'll find that replies don't always work.

jdanish commented 4 months ago

OK thanks. So does that work as noted above for a relatively easy first pass to then let us do some testing?

  1. If there are no replies, remove it and then collapse anything below up.
  2. If there are replies, warn before removal, but then remove all replies as well.
benloh commented 4 months ago

So does that work as noted above for a relatively easy first pass to then let us do some testing?

Yes, I believe so. We should be ready to do a more robust test hopefully by next week. But I'm running into some gnarly async update issues, so it might take a little longer.

benloh commented 4 months ago

Sixth Pass

Comments should now be saved to the database.
Reply threading logic should also work as expected (before this fix, a reply to a second comment would either bomb out or clobber existing threads).

jdanish commented 4 months ago

If you select a node and click the new comment button then a new comment box appears. Drag it around and no surprises. However, if you first open the nodes table then click the new comment button, same box appears, but when you drag it you discover a second one is hidden behind the first.

jdanish commented 4 months ago

I assume yellow means unread and gray means read? It looks like read status is not preserved across a window reload.

benloh commented 4 months ago

If you select a node and click the new comment button then a new comment box appears. Drag it around and no surprises. However, if you first open the nodes table then click the new comment button, same box appears, but when you drag it you discover a second one is hidden behind the first.

Yeah that's a little funny. I'll see if I can close the first before opening the second.

I assume yellow means unread and gray means read?

Yes, and also there's a red dot. screenshot_1445

A comment is marked "read" when you click "Close", but not if you click away. We can change that.

It looks like read status is not preserved across a window reload.

Ah, good catch. I'm not saving that to the database yet.

benloh commented 3 months ago

Seventh Pass

At last! I think we're ready for testing. There was a lot to manage. Main changes:

Feature Updates

Technical Updates

@jdanish Ready for testing!

jdanish commented 3 months ago

Not 100% sure how I triggered this. The top comments were created using http://localhost:3000/#/edit/1-1-68M?admin=true and the new one with http://localhost:3000/#/edit/1-1-GNY (the one that says "and more"). I looked at the console because the cancel button wasn't doing anything. That's the error. If I save, I can then delete it later. But the cancel won't work for now. Cancel was previously working. It happens consistently now that I am in this state, but I was tinkering for 10 minutes to get here. Ideas on things to test?

Screenshot 2024-03-29 at 7 34 46 AM

UPDATE: this happens on a new network too. So the steps above are irrelevant.

jdanish commented 3 months ago

Update, this now happens in both accounts for any new comments that I try to cancel. I did use the admin power to delete a thread before this began happening in case that helps. Restarting the app does not change anything.

benloh commented 3 months ago

Yeah that was the "chasing a few minor things" I was talking about. Hope to get that resolved by tomorrow.

jdanish commented 3 months ago

Yeah that was the "chasing a few minor things" I was talking about. Hope to get that resolved by tomorrow.

All good! Looking good so far, though ramping up :)

Chatting with the team here if you want to see. I'll share anything we agree on as an issue or comment here, but you are welcome to preview / comment: https://docs.google.com/document/d/1q3MPP7NQR9uzdj0Jl3eCOXEumRP9yaoXfo7gzbjwOXI/edit#bookmark=id.6ysrq5fjv1cq

benloh commented 3 months ago

"Cancel" a comment should work properly now.

jdanish commented 3 months ago

When I test, if I create a comment then hit cancel it is saving an empty comment. Then I can delete it. So no error like before, but also not deleting it?

Screenshot 2024-03-29 at 8 54 15 PM

UPDATE: If I literally just hit create then cancel, it deletes it. But if you type a letter and then hit cancel it erases the comment but saves it where I'd expect it to be deleted. Also I would think the cancel either doesn't throw a "delete" warning, or it is a different one ("Do you want to cancel?") -- preferably the latter?

benloh commented 3 months ago

Ack. Sorry. I had to the change the logic to address restoring the original value if you canceled but missed this case.

benloh commented 3 months ago

Who knew that canceling would be so complex. The new logic:

But...

jdanish commented 3 months ago

OK! That appears to work for now!

benloh commented 3 months ago

Addressed with #114