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.

Sign up for GitHub

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

Jump to bottom

MBS-12841 / MBS-8242: Link to edit and relationship docs from edits #2876

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

MBS-12841 / MBS-8242: Link to edit and relationship docs from edits #2876

wants to merge 5 commits into from

Conversation

reosarevok
Copy link
Member

@reosarevok reosarevok commented Mar 1, 2023 •
edited

Implement MBS-12841, MBS-8242

Problem

Edit listings could always benefit from more ways to help voters make the right choice. Right now, there's two pieces of documentation we have and would almost certainly be good to show, but we are not linking in any way. One is the edit type documentation for the edit (the data there is not great, but it's being improved with #2784 and should be improved further by actually documenting each edit type). The other is the documentation for relationship types involved in relationship edits (which usually have guidelines! which we don't have a way to access from the edit!).

Solution

The second commit here adds a link to the doc page for the specific edit type in question to the header of every edit, be it on an edit list or on the edit page itself. This replaces the generic "Edit Types" link on the sidebar (not shown on edit lists) that I remove in the first commit.

The last commit adds a link to the doc page(s) for the relationship type(s) involved in a relationship edit (add, edit or remove). Rather than making it float by each relationship, I put it on the top right of the changes display, since @Aerozol suggested we should aim to have help boxes on the top right.

While testing that commit I found that types weren't being loaded properly for relationship removals - fixed with the third commit.

image_2023-03-01_184431942

Testing

Manually, using a bunch of relevant edits that happened to already be on my local sample data history from my previous tests for other PRs. A good testing suggestion would honestly just be to play with edit searches, but make sure you navigate to the edit pages as well to check the help works there too.

@reosarevok reosarevok added the QoL Non-urgent quality of life improvements label Mar 1, 2023
@yvanzo yvanzo marked this pull request as draft April 12, 2023 07:55
@yvanzo
Copy link
Contributor

yvanzo commented Apr 12, 2023

Didn’t review yet, just noted that it now has conflicts.

@reosarevok reosarevok marked this pull request as ready for review April 12, 2023 14:48
yvanzo
yvanzo requested changes Jul 26, 2023
View reviewed changes
Copy link
Contributor

@yvanzo yvanzo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Linking to the documentation is a must have, thank you for dealing with this.

At first glance, there are some issues with the current submission:

  1. It doesn’t seem helpful to remove the link to the documentation for all edit types in the sidebar.
  2. When an edit is replacing a relationship by another one of a different type, it is not clear which relationship type the help is about. There should probably be two help buttons in such case.
  3. The help icon is too far away. It should probably just be by the text as in the external links editor.
  4. The text in the tooltip should directly show the type’s description as in the external links editor.

Also do not hesitate to ask our design consultant some input on this.

@yvanzo
Copy link
Contributor

yvanzo commented Jul 26, 2023

since @Aerozol suggested we should aim to have help boxes on the top right.

Just reading this. I still think that icons are too far. @Aerozol what did you think about this issue?

@reosarevok
Copy link
Member Author

I'll leave 3 and 4 to Aerozol :)

  1. What is the use of linking to a generic list when we already link to the actual relevant type? (from where you can get to the generic list with one more click if needed)

  2. Did you test such edits? :) The help / tooltip shows something like "See the documentation for the relationship types in this edit: “member of band”, “sibling”." (with two links)

@yvanzo
Copy link
Contributor

yvanzo commented Jul 26, 2023 •
edited

  1. What is the use of linking to a generic list when we already link to the actual relevant type? (from where you can get to the generic list with one more click if needed)

Saving one click. The sidebar is not crowded either.

  1. Did you test such edits? :)

Not this exact kind of edit, I trusted your screenshot. I also had 4. in mind.

@reosarevok
Copy link
Member Author

I just don't see when that main list is ever relevant to any specific edit, honestly :)

@Aerozol
Copy link
Contributor

Aerozol commented Jul 28, 2023

Thanks both!
3. I agree that on its own the help icon looks far away from the text in question, but I think long-term it would be user-friendly to have all the help/doc materials on the right. There is room there, so we can be quite detailed with what we eventually add, and users will eventually know where to look. Here’s a professional mockup of what I mean:
image
3b. Perhaps custom styling the [?] box to fit in with the background a bit more, and maybe be a bit bigger, would help? So it’s not ‘floating’ on it’s own as much. Happy to deliver some designs.
4. I’m not sure I can see what’s in the tooltip currently - this isn’t on test or beta is it, for me to test?

@yvanzo
Copy link
Contributor

yvanzo commented Jul 28, 2023 •
edited

I’ve just put this branch on test.mb.o. Here are a few example pages to look into:

Note that there is no such help button in relationship editor at the moment.

@Aerozol
Copy link
Contributor

Aerozol commented Jul 28, 2023

Thanks yvanzo!
I don’t mind it on the right, but this situation is definitely confusing (and a bit ugly):
image

Maybe it is worth spending longer on the design… something like:
image

  1. yes, it would be to have: “See the documentation for this edit type: [Edit relationship]“, instead of just “See the documentation for this edit type”

@reosarevok
Copy link
Member Author

it would be to have: “See the documentation for this edit type: [Edit relationship]“, instead of just “See the documentation for this edit type"

Hmm. I was only showing the types for relationships, because for edits there's only ever one type per edit and the type name is in large font on the heading. But if you feel it's useful for that, we could add it?

@Aerozol
Copy link
Contributor

Aerozol commented Sep 6, 2023

Thanks reo!
re. 4, my gut feeling is that even with the text repeated, it is good practice to let someone see where a link is sending them. Maybe it will be annoying - I haven’t mocked this up and imagination is never the same as the real thing. So I’m leaning towards having the edit type in the rollover.

@yvanzo
Copy link
Contributor

yvanzo commented Sep 6, 2023 •
edited

You missed the point 4. What is in the tooltip for relationship type in the external links editor is the description of the relationship type, not just its name, so the user doesn’t even have to click the link in most cases.

@reosarevok
Copy link
Member Author

I did miss that! I would like people to actually go and read the guidelines too, but it might be good to show more here - that just means we need to load it first though since I guess it's not loaded at the moment (unlike when editing).

@reosarevok
Drop generic Edit Types doc link from edit sidebar
9b64949 
This is only shown when viewing one edit at a time, and does not point
to docs for the specific edit either, but to a generic page.
It should be replaced by a documentation link that is visible
from the edit header on any edit view.
@reosarevok
MBS-12841: Link to specific edit type docs from edits
2b5bf76 
This adds a link to the doc page for the specific edit type in question
to the header of every edit, be it on an edit list or
on the edit page itself.
This replaces the generic "Edit Types" link on the sidebar
(not shown on edit lists) that was removed in the previous commit.
@reosarevok
Properly load LinkType for Relationship::Delete
309f008 
This was probably copied from Create or other place where the structure
was different, but here {link} is inside {relationship}.
@reosarevok
Move getRelationshipLinkType to /edit
8472078 
I'm using this outside the relationship editor in the next commit,
so it seems cleaner to move it here.
@reosarevok
Copy link
Member Author

reosarevok commented Mar 14, 2024 •
edited

Changed this to actually show more info about the relationships as requested by @yvanzo (seems like a good call):

example

It also now does "See the documentation for this edit type: Edit relationship." on the edit type popup, as requested by @Aerozol.

The UI still seems kind of atrocious, though. @Aerozol, wanna play with it? :) (I understand you can now run code locally).

@reosarevok
MBS-8242: Link to specific relationship type docs from rel edits
652523f 
This adds a link to the doc page(s) for the relationship type(s)
involved in a relationship edit (add, edit or remove),
and lists the type descriptions for a basic overview.
It seems very useful to have direct links to the documentation
(often including specific guidelines)
directly in the place people need to vote on the changes from.

We pass an array to allow multiple types in EditRelationship;
we pass the rels through a Set to remove duplicates
when EditRelationship does not change the rel type.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yvanzo yvanzo yvanzo requested changes

Assignees
No one assigned
Labels
QoL Non-urgent quality of life improvements
Projects
None yet
Milestone
No milestone
3 participants
@reosarevok @yvanzo @Aerozol