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

"Actions" drop down options do not give context to the user #6301

Merged
merged 14 commits into from
May 26, 2020
Merged

"Actions" drop down options do not give context to the user #6301

merged 14 commits into from
May 26, 2020

Conversation

BatJan
Copy link
Contributor

@BatJan BatJan commented Sep 7, 2019

Prerequisites

  • I have added steps to test this contribution in the description below

Addresses issue 9 and 45 from #5277

Description

This issue is about providing a better non-visual context to the available menu options from the "Actions" dropdown
actions-menu-open

Currently what a screen reader will read out is simply the names so I will for instance read "Create", "Delete" etc. - But it may not be enough context. It would probably be better to have it read "Create a new node", "Delete this node" and so forth.

Therefore I was wondering if it would make sense to extend the menuItem object returned to the frontend so it contains the properties "textBefore" and "textAfter" and then also extending the language files with for instance "create_before" and "create_after" making it possible to add non-visual text before and after the item for screen readers.

I have updated the language files for en, en_us and da in this PR.

The C# changes was made using my editor but is primarily the work of @rasmusjp who was kind enough to help me out while i was playing around with this 🙌

So I realize that HQ might have a desire to deal with this in a different fashion - So I'm open for a discussion around how to best go about doing this if the suggested approach is not ideal or have downsides I have not thought about etc.

I was also wondering if perhaps a simpler approach would be to handle the fecthing of the "_before" and "_after" texts directly from the frontend instead since the dictionary alias is provided in the returned menu object already. Then some kind of service for getting the needed texts for the non-visual texts could be created instead maybe.

I have added "before" and "after" items, since depending on language and context it's needed to be able to construct the texts differently. For instance the "Culture and Hostnames" setting makes sense to have read out like "Setup Culture and Hostnames for this node"... but in danish it's "Tilføj domæne", which makes sense to have read out like "Tilføj domæne på denne node".

I'm looking forward to receiving some feedback and thoughts for this concept 😃

# Conflicts:
#	src/Umbraco.Web.UI.Client/src/views/components/editor/umb-editor-menu.html
#	src/Umbraco.Web.UI/Umbraco/config/lang/en.xml
#	src/Umbraco.Web.UI/Umbraco/config/lang/en_us.xml
@nul800sebastiaan
Copy link
Member

This one is interesting and I am not sure about the proposed solution. Maybe a better solution would be to have (for example):

        <key alias="assignDomain">Culture and Hostnames</key>
        <key alias="assignDomain_description">Setup culture and hostnames for this node</key>

That way, it will also be possible to have consistent capitalization and you're not constrained to the order of words, which you already noticed changes in some languages depending on what you're trying to express.

As an added bonus the _description translation can become a tooltip as well, clarifying the menu options to people who don't have visual impairments,

@BatJan
Copy link
Contributor Author

BatJan commented Sep 15, 2019

@nul800sebastiaan Yes, that' probably a better way to deal with it. Makes it less confusing to deal with indeed.

So I suppose that the textBefore and textAfter should be changed so instead of exposing these properties we will expose the description property, correct? 😃

I like the idea about using the description as a tooltip in a visual context as well - I'm thinking that you're thinking that a title attribute containing the description should be added to the item? If so I'm afraid that it might not be a good idea since how browsers and screen readers treat the attribute is very different. The accessibility advent calendar has an article from 2017 about it here https://www.24a11y.com/2017/the-trials-and-tribulations-of-the-title-attribute/

Personally I like the tooltip idea but I think that we might need to consider doing some kind of custom tooltip using a custom data-tooltip attribute that will contain the text and then we can visually add a tooltip on hover... But I think that's out of scope for this PR since it's a new feature in itself that will require some calculations to figure out how and where to display the tooltip unless we can find a suitable 3rd party plugin that would be OK to use for instance 😃

But this should not stop us from progressing on this PR - Once the initial work here is done we can always use the description for a visual tooltip later on 👍

@BatJan
Copy link
Contributor Author

BatJan commented Sep 15, 2019

@nul800sebastiaan I just had a quick chat with @zersiax about this and he confirmed that the use of the title attribute is problematic

@zersiax
Copy link
Contributor

zersiax commented Sep 15, 2019

Hmm ...this is a tricky one.
Context is a problem in many places throughout the Umbraco back office, for example, when adding properties to a new group in a document type, so I think a generic strategy for fixing that is a good idea.
This might be grasping at straws here, but can we use computed values for some of this extra explanatory text?
My thinking is that if you are opening the action menu for a node, the fact that you are working with a node or even what kind of node is somewhere in the variables we have access to at that point already. Can we do string interpolation within the translations where we do something like "Create new {{ currentItemType}}" or something to that extent?
Or even an announcement what menu we are opening when it opens? At that point, you won't need to change all the menu options, you just need to add an announcement for what menu the following options pertain to, either on the button that opens the menu, or some kind of alert when the menu opens. Just my two cents for if alternatives are needed :-)

@BatJan
Copy link
Contributor Author

BatJan commented Sep 15, 2019

@zersiax The challenge with using a computed value is that it will not make sense for all of the possible action. The actions are looped over in the frontend code so it's not possible to differentiate the passed values. So while using the name of the document type in the "Create" action would be fine it would make less sense when using the "Delete" action for instance... here it would make more sense to use the name of the node so it would read "Delete {{currentNodeName}}" for instance.

Hmm, but thinking about it... Maybe we could extend the object returning the actions having a "useDocumentTypeName" and if it's true then we know if we need to either use the document type name or the node name. But I think we will still need to use a description like @nul800sebastiaan suggested since we will need to be able to have nice text alternatives like "Create new {{documentTypeName}}" and "Set permissions for {{nodeName}}" for instance. Unless it would be enough to have something like "Create - {{documentTypeName}}" and "Permissions - {{nodeName}}" for instance?... The examples are based on 2 out of 10 possible actions. I'm trying to illustrate that depending on the name of the action the relevant description and context differ 😃

@RachBreeze
Copy link
Contributor

RachBreeze commented Sep 16, 2019

Hi @BatJan and Florian I've been dealing with something similar around the name of nodes for both the content section (#6236) and the rest of the site #6315 (the rest of the site uses the umbHeaderDirective), it strikes me we are possibly tackling the similar issues from different angles

@BatJan
Copy link
Contributor Author

BatJan commented Sep 16, 2019

@RachBreeze Thanks for chiming in - Yes that might very well be the case. Currently I'm very keen on @nul800sebastiaan suggestion about "_description" in the dictionary files but without using the title attribute for the visual context but something custom to show a tooltip that will not interfere with screen readers etc.

However I'm also open for the suggestions from @zersiax using the computed values if we can make it work in a sensible manner. Then that's my preferred approach 👍

@zersiax
Copy link
Contributor

zersiax commented Sep 17, 2019

If I need to a-b test anything here, feel free to let me know. I don't quite know enough about Umbraco's internals yet to comment further on what would be a doable/sensible approach when it comes to technical feasibility :-)

# Conflicts:
#	src/Umbraco.Web.UI/Umbraco/config/lang/da.xml
#	src/Umbraco.Web.UI/Umbraco/config/lang/en.xml
#	src/Umbraco.Web.UI/Umbraco/config/lang/en_us.xml
@BatJan
Copy link
Contributor Author

BatJan commented Sep 29, 2019

Just a heads up - This PR is not ready for being merged yet. Still some work to be done 😄

BatJan added 2 commits October 7, 2019 18:27
# Conflicts:
#	src/Umbraco.Web.UI/Umbraco/config/lang/da.xml
#	src/Umbraco.Web.UI/Umbraco/config/lang/en.xml
#	src/Umbraco.Web.UI/Umbraco/config/lang/en_us.xml
…nly have descriptions for those that need it
@BatJan
Copy link
Contributor Author

BatJan commented Oct 7, 2019

@nul800sebastiaan Ok, this PR is ready for review 😃

I have changed the approach so I'm using "_description" as suggested in combination with the computed value for the nodename.

So for some of the actions there will be a description and for some of them there will not.

For instance the "Create" action will have a hidden text that says "Create new node under Home" where "Home" is the name of the current node.

On the other hand the "Delete" action will have a text that says "Delete Home" - Again "Home" is the name of the currentNode.

Looking forward to receive any further comments on this one 👍

# Conflicts:
#	src/Umbraco.Web.UI/Umbraco/config/lang/da.xml
#	src/Umbraco.Web.UI/Umbraco/config/lang/en.xml
#	src/Umbraco.Web.UI/Umbraco/config/lang/en_us.xml
@BatJan
Copy link
Contributor Author

BatJan commented Nov 7, 2019

@nul800sebastiaan OK! SO now this one is ready for your final review - I have done the changes that you requested so now we have magic strings and all! 🎊 😃

@BatJan
Copy link
Contributor Author

BatJan commented Apr 8, 2020

@nul800sebastiaan Is there anything that I need to fix in this PR? Otherwise I think it should be good to go 😃

BatJan and others added 2 commits April 10, 2020 10:19
# Conflicts:
#	src/Umbraco.Web.UI/Umbraco/config/lang/da.xml
#	src/Umbraco.Web.UI/Umbraco/config/lang/en.xml
#	src/Umbraco.Web.UI/Umbraco/config/lang/en_us.xml
@nul800sebastiaan nul800sebastiaan changed the base branch from v8/dev to v8/contrib May 26, 2020 13:58
@nul800sebastiaan nul800sebastiaan merged commit 32a0c3b into umbraco:v8/contrib May 26, 2020
@nul800sebastiaan
Copy link
Member

Sorry for the long delay on this one @BatJan - finally got around to checking this one out and it's great, thanks very much! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants