grommet / hpe-design-system

HPE Design System
48 stars 24 forks source link

Managing Child Objects - revisit placement of delete control #3177

Open ericsoderberghp opened 1 year ago

ericsoderberghp commented 1 year ago

Questions have come up regarding the placement of the delete control for a managed child object. Especially if the FormField above it is numeric and shorter, that leaves the delete icon sort of hanging out too far away from the rest of the object's fields.

Originally a discussion on Slack: https://grommet.slack.com/archives/C04LMJWQT/p1677599590742679

Also a discussion in Office Hours.

ericsoderberghp commented 1 year ago

Proposed alternative example:

Image

ecircenis commented 1 year ago

Consider this design and how other potential delete icons within a child object could cause problems with not giving the main delete icon affinity to what is being deleted. https://designer.grommet.io/?id=Child-objects-edgar-circenis-hpe-com

halocline commented 1 year ago

Consider this design and how other potential delete icons within a child object could cause problems with not giving the main delete icon affinity to what is being deleted. https://designer.grommet.io/?id=Child-objects-edgar-circenis-hpe-com

A couple thoughts regarding this example: 1) I agree the positioning of the trash actions is problematic in this example. 2) Managing three generations of objects within a single view seems problematic. 3) The grandchildren in this example only have two inputs. What happens if the grandchild objects have more than two fields? How does the layout/pattern adjust so that it is flexible to accommodate other data structures which have a greater number of properties? 4) The attribute input paired with a value input seems to be functioning more as a key-value pair where a property and value are being specified. And the property-values are being added/removed from the object. Perhaps we should be evaluating an input pattern which suits this key-value pair add/edit/remove instead of thinking of these as child objects?

ecircenis commented 1 year ago

From a consistency perspective, the way we use the trashcan is very consistent across our designs:
[item] (trashcan). The trashcan is always to the right of the thing being removed and is top aligned with it. We should do the same here. Just because the item can be expanded/contracted should not influence the application of the pattern of the trashcan being in the upper right.

jeffstolz commented 1 year ago

Jumping in as I'm taking over this ticket. While there's a debate around the best placement of the button and possible over-nesting of objects within this view, I think we need to trust the service design team in their choice based on the knowledge and ownership of their context. That being said, @ecircenis and the team are working off assumptions on how the user is interacting with this tool, so it will be on them to identify any pain points through user testing, modify the approach if they arise, and communicate that back to the design system team. Other than that, I don't think we should be debating their usage of this within the services, but should be asking "Should we standardize their decision within the design system?"

My recommendation would be no for now - until we validate the approach with users and identify the reusability of this pattern across our experience, we punt on updating documentation on the accordion component and move on to higher-priority issues.

Thoughts everyone?

ecircenis commented 1 year ago

@jeffstolz The affinity between the trashcan and the thing being deleted is non-existent, and is also inconsistent with how the trashcan is used everywhere else. By choosing to not make changes in DS, while both agreeing that the current design is bad and also trusting that the GLCS team is making the right decision, the only option available to GLCS is to not use the DS component at all. That hardly seems like the correct approach. Why not fix what we know is broken/inconsistent?

jeffstolz commented 1 year ago

@ecircenis I'm coming from the perspective of shifting our strategy towards a more flexible design system, which I understand is outside the context of this issue and another conversation. To be clear - I'm not agreeing the current design is "bad" and there is not a "non-existent" affinity between the object and the trashcan, it's a parent/child relationship. We can include the designs Chandan provided as an option to use when you want the user to be able to easily delete the entire object (the context typically seen within the services), but I wouldn't want to mandate all teams to follow that as a required pattern or restrict either pattern from use.

ericsoderberghp commented 1 year ago

Unless there does not appear to be a valid reason for the discrepancy. One of our goals is to ensure users do the same things in the same way across apps wherever possible.

jeffstolz commented 1 year ago

Met with Chandan and saw their updated design within the services which I think is a stronger approach than both options presented earlier.

Add quotas

After further thought, I recommend we update our current documentation to reflect this single option. I've looked into Outlook, Gmail, and Slack and, while the cases are slightly different, they more closely represent this pattern.

This also addresses @ecircenis concern about the weak affinity - by having the delete horizontally aligned to the heading of the parent object it's certainly a closer association. If there is concern about the trash icon being too easily accessed for consequential actions, the action could be confirmed with a dialog.

If the team agrees, I can create a Figma template for this (none currently exists) and submit a PR to update the web guidance.

ericsoderberghp commented 1 year ago

The trash can icons seem to become the visual anchor for demarcating the children, given that the horizontal rule doesn't extend all the way to the right, and seem like they might be too prominent to me. I'm fine with an upper placement but wondering if they can be de-emphasized a bit more somehow. Just a quick reaction upon seeing the visual.

jeffstolz commented 1 year ago

Looping in @vavalos5 who brought up the point that, by emphasizing the delete action in the pattern, it could more easily allow for misclicks. I think both the current and proposed approaches are going to have their tradeoffs, and it really is up to the designer to choose the best option for their context. Perhaps we allow for both options and clearly define the correct use cases for each? I don't want to be overly prescriptive and override the design decision of the service team: if the pattern doesn't meet their specific user needs, I'd expect them to abandon the DS component over abandoning their solution (as @ecircenis said).

jeffstolz commented 1 year ago
Screen Shot 2023-05-15 at 9 18 04 AM

Took some time to explore some upper placement options before committing to this. Each has its tradeoffs, although I recommend Option 2 overall.

Option 1 - What Chadan proposed and already exists in their design. While the upper placement no longer hides the trash icon, as @ericsoderberghp said, it overrides the visual anchor (which should be the caret since it's actually referencing the children).

Option 2 - My recommendation. Serves the purpose of what @ecircenis and Chandan wanted by reinforcing a stronger affinity, but doesn't side outside the HR or over-emphasize itself. What do you think, Edgar?

Option 3 - An exploration, though not recommended. It is both too subtle and requires additional actions by the user. If we wanted it to be this subdued, the current pattern of nesting the trash under the body.

jeffstolz commented 1 year ago
Screen Shot 2023-05-15 at 9 18 04 AM

Took some time to explore some upper placement options before committing to this. Each has its tradeoffs, although I recommend Option 2 overall.

Option 1 - What Chadan proposed and already exists in their design. While the upper placement no longer hides the trash icon, as @ericsoderberghp said, it overrides the visual anchor (which should be the caret since it's actually referencing the children).

Option 2 - My recommendation. Serves the purpose of what @ecircenis and Chandan wanted by reinforcing a stronger affinity, but doesn't side outside the HR or over-emphasize itself. What do you think, Edgar?

Option 3 - An exploration, though not recommended. It is both too subtle and requires additional actions by the user. If we wanted it to be this subdued, the current pattern of nesting the trash under the body.

To be clear - recommend this as a variant to our current guidance, not in place of it. We would articulate the proper usage of each in the guidance.

ecircenis commented 1 year ago

I like #2. Have you also considered a variant of #2 where the arrow and trashcan order are swapped (like #1, but with the border extending all the way out like #2)? (I could go either way)

jeffstolz commented 1 year ago
Screen Shot 2023-05-15 at 10 03 10 AM

@ecircenis Adding it in as option 4 - looking at it, I think it's better than Option 1 but Option 2 is still the best - there is a distinction to the furthest right aligned item in a row like this that I still places the visual focus on the trash icon. If Edgar is cool with Option 2, what do you both think @vavalos5 and @ericsoderberghp? I can start updating the guidance if we're all in agreement.

jeffstolz commented 1 year ago

Dropping in feedback from @SOjaHPE - TL;DR - agrees with Option 2, and like its ability to scale

https://hpe.slack.com/archives/GPPLUK6LR/p1684159878039629

ericsoderberghp commented 1 year ago

One concern I have with all options is that typically the entire accordion header is a click target to open/close the panel. With the options above, the boundaries of where the delete click target ends and the accordion panel begins are unknown, so it could lend itself to stray clicks. Also, we should not be nesting interactive elements as that causes accessibility issues. The browser will complain if buttons are nested inside each other. I think we should look for a more accessible way to address the request.

ecircenis commented 1 year ago

The simple solution is: don't nest them. Don't make the entire header clickable, but only the two controls in the header. Otherwise, reconsider the original #1 proposal where the trash can is outside of the accordion and this entire pattern is just creation of a list of side-by-side accordion elements and trash buttons. After all, the trash button deletes the thing represented by the accordion item. That's super consistent with how trash icons typically work -- they delete the item to the left.

jeffstolz commented 1 year ago

One concern I have with all options is that typically the entire accordion header is a click target to open/close the panel. With the options above, the boundaries of where the delete click target ends and the accordion panel begins are unknown, so it could lend itself to stray clicks. Also, we should not be nesting interactive elements as that causes accessibility issues. The browser will complain if buttons are nested inside each other. I think we should look for a more accessible way to address the request.

Thanks, @ericsoderberghp - good to know, that changes things for sure. Is Option 1 considered accessible, as the trash icon sits to the right of the accordion header and it remains an unbroken click target? Not necessarily asking right now if it's the best design decision, but can we consider it an accessible solution? I'd like to remove all options that introduce accessibility concerns from our list of choices.

It seems all other options from that list need to be removed.

jeffstolz commented 1 year ago

The simple solution is: don't nest them. Don't make the entire header clickable, but only the two controls in the header. Otherwise, reconsider the original #1 proposal where the trash can is outside of the accordion and this entire pattern is just creation of a list of side-by-side accordion elements and trash buttons. After all, the trash button deletes the thing represented by the accordion item. That's super consistent with how trash icons typically work -- they delete the item to the left.

@ecircenis I responded before seeing your reply - but option 1 may make the most sense. Wouldn't we want the entire header to be clickable? Isn't option 1 as you presented it be the only choice that is accessible?

jeffstolz commented 1 year ago

We may want to turn this into a call, the conversation is active and I'm getting a bit mixed up with the replies

jeffstolz commented 1 year ago

After discussions with the cloud services team and show & tell, we'll be moving forward with Option 1 "Top level placement" as an alternate to the more standard current placement which we can call the "Nested placement". Here's the updated guidance section I plan to add to the site:

Remove Parent Object For use cases where the user needs the ability to remove the parent object entirely, there are two options available.

Nested placement The nested placement is the standard option for removing a parent object. It avoids competing with the visual anchor of the object header and prevents users from accidentally removing the object. Always position the remove button to the bottom right of the body.

Top-level placement

This is an alternate option to be used for specific use cases. Consider using this option when you are concerned that the nesting of the remove action can be lost in the user experience. This can occur if the user is expecting the remove action to be higher in the visual hierarchy due to it being an expected/priority action, such as when editing and manipulating a collection of objects. It can also occur when an object body has a large amount of content and the remove action can get lost below the complexity. Note: if this is the case, ensure that the accordion is the best pattern to be using in this context. Accordion best practices can be found here. Ensure that the remove action is horizontally aligned with the object header, positioned to the right.

jeffstolz commented 1 year ago

After discussions with the cloud services team and show & tell, we'll be moving forward with Option 1 "Top level placement" as an alternate to the more standard current placement which we can call the "Nested placement". Here's the updated guidance section I plan to add to the site:

Remove Parent Object For use cases where the user needs the ability to remove the parent object entirely, there are two options available.

Nested placement The nested placement is the standard option for removing a parent object. It avoids competing with the visual anchor of the object header and prevents users from accidentally removing the object. Always position the remove button to the bottom right of the body.

Top-level placement

This is an alternate option to be used for specific use cases. Consider using this option when you are concerned that the nesting of the remove action can be lost in the user experience. This can occur if the user is expecting the remove action to be higher in the visual hierarchy due to it being an expected/priority action, such as when editing and manipulating a collection of objects. It can also occur when an object body has a large amount of content and the remove action can get lost below the complexity. Note: if this is the case, ensure that the accordion is the best pattern to be using in this context. Accordion best practices can be found here. Ensure that the remove action is horizontally aligned with the object header, positioned to the right.

This would include examples, just articulating the language here

jeffstolz commented 1 year ago

@halocline I imagine you'd want to weigh in on this - I'm working on the PR for this and considering the structure. Currently, we have the remove button placement shown within the anatomy diagram. I'm considering removing it from the diagram since we now present two options, and articulating each option as examples.

My question: now that we have two options for remove button placement, is it appropriate to no longer consider it's placement part of the core anatomy? Since the ability to remove a parent object is optional anyways, is this really an example use case?

If not, we can provide two options for the core anatomy of the template, but I'm concerned that would be confusing.

jeffstolz commented 1 year ago

Draft PR with updated guidance https://github.com/grommet/hpe-design-system/pull/3365

Will be working with @jcfilben to button up this PR and submit it once the example is created.

jeffstolz commented 1 year ago

PR up for review https://github.com/grommet/hpe-design-system/pull/3365

taysea commented 1 year ago

6/14: @halocline to revisit if this should still be priority or revisited in subsequent sprint. Work has been done, need to decide on direction if we want to support or not.