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

Tag: when a Tag is actionable(has an onClick), the pointer icon does not change #7592

Closed
1 of 2 tasks
guigueb opened this issue Jan 19, 2021 · 23 comments · Fixed by #7839
Closed
1 of 2 tasks

Tag: when a Tag is actionable(has an onClick), the pointer icon does not change #7592

guigueb opened this issue Jan 19, 2021 · 23 comments · Fixed by #7839
Assignees
Labels
component: tags proposal: open This request has gone through triaging. We're determining whether we take this on or not. type: bug 🐛

Comments

@guigueb
Copy link
Contributor

guigueb commented Jan 19, 2021

Title line template: Tag: when a Tag is actionable(has an onClick), change the pointer icon

What package(s) are you using?

  • carbon-components
  • carbon-components-react

Detailed description

When a Tag has a filter property, the mouse icon will change when hovering over the Tag.
<Tag type="red" title="Clear Filter" filter> Red </Tag>
If a Tag does not have the filter property but does have an actionable property(onClick), the mouse will NOT change when hovering over the Tag.
<Tag type="red" title="Clear Filter" onClick={()=>alert("red")}> Red </Tag>

I expected the mouse icon to change when actionable in the same way as when filter property is set.

This happens in any browser.
This happens in any version of the Carbon Design System.

Steps to reproduce the issue

  1. Create a tag like <Tag type="red" title="Clear Filter" filter> Red </Tag>
  2. Create a tag like <Tag type="red" title="Clear Filter" onClick={()=>alert("red")}> Red </Tag>
  3. Hover over each and observe the icon changing(or not).

Use case

The product has a search input.
The product has three suggestions for searching "Platform", "Runtimes", and "Services".
It shows these as clickable tags for quick filtering.
This is on the Carbon Design System Tag page just above the Live Demo.
https://www.carbondesignsystem.com/components/tag/usage#live-demo

Work arounds

The child of Tag is a node, so it could be coded as an input.
Styling could be applied, to account for the hover.
Always create a filter tag - and apply display: none; to the filter button when not a filter tag.
All of these are client work and they come with their own problems.

@dakahn
Copy link
Contributor

dakahn commented Jan 20, 2021

confirmed the above described behavior.

@dakahn
Copy link
Contributor

dakahn commented Jan 20, 2021

@carbon-design-system/design is the intention that tags are used like buttons as described above?

@guigueb
Copy link
Contributor Author

guigueb commented Jan 20, 2021

Yes, exactly - we display tags which have an onClick action similar to buttons.
Our usage looks similar to the tags in the "Data table" and "search input" on the https://www.carbondesignsystem.com/components/tag/usage page.
Where clicking on a tag will perform an action like showing properties related to a tag, or filter by a tag, etc.
This behavior currently works great - but the icon doesn't indicate the tag is actionable.

@emyarod
Copy link
Member

emyarod commented Jan 21, 2021

cc @carbon-design-system/design does the default Tag support click actions or is that specific to Filter Tags only?

@dakahn
Copy link
Contributor

dakahn commented Jan 21, 2021

this was briefly discussed on Slack, seems like a proposal/feature enhancement to me. Converting and moving to the right pipeline

@guigueb
Copy link
Contributor Author

guigueb commented Jan 21, 2021

Any Tag (default or with a filter prop) supports and actionable callback (like onClick).
A Tag with a filter prop....

  • changes the mouse icon indicating it is actionable
  • as it is actionable it is expected to have an actionable callback (like onClick) which will remove the tag or filter prop

I would suggest that the mouse icon change should be tied to having an actionable callback (like onClick) not to the flag filter prop.

@dakahn
Copy link
Contributor

dakahn commented Jan 21, 2021

@guigueb yeah, although onClick works for non-filter tags, it seems like from a design spec standpoint, that's not a intended use case at the moment.

Hover, active, focus states need to be determined etc etc. 👍🏽

@guigueb
Copy link
Contributor Author

guigueb commented Jan 21, 2021

@dakahn

Shouldn't all the hover, active, focus states already be in place for a filter tag - as it is an actionable tag?
I was hoping/expecting to simply apply any/all the same styling for actionable tags as you would for filter tags.

@dakahn
Copy link
Contributor

dakahn commented Jan 21, 2021

the states are in place for the filter variant which has specific use cases and guidance. As it stands a clickable action on a non-filter tag isn't something we support from a design system standpoint even though as you point out the API seems to support this behavior.

@guigueb
Copy link
Contributor Author

guigueb commented Jan 21, 2021

OK.
A work around is to create a filter tag - and apply display: none; to the filter button.

@tw15egan
Copy link
Member

This also goes hand in hand with #7160, as click events are passed directly to the Tag through the ...other props.

@PaulJThompson
Copy link

In the prior version of Cognos Analytics, the cursor does change when a tag has an action associated with it. From our point of view, it is a regression that Carbon does not support this.

@aagonzales
Copy link
Member

The default tag should continue to not have a cursor change. However, we can add a new variant for interactive tag that has a hover background color change and a cursor change.

I'm curious what the use case for this is and what other states might be needed. Does there also need to be a selected state? What kind of actions are you attaching to the tag?

@guigueb
Copy link
Contributor Author

guigueb commented Feb 25, 2021

agreed - The default tag should continue to not have a cursor change.

I'm not sure a new variant is needed - just have the tag be aware of any onXXX events it may have.
(I'm pretty sure the filter prop is not needed - you could have just checked for an onClose callback)
If the tag contains a onClick, onClose, etc... then its actionable and the Carbon states (hover/focus/etc) should apply.

We don't use a selected state... but I suppose that could be an enhancement.
Selected state would require additional props.

I am only aware of using the actions onClick and onClose, but who's to say what users will add.
But now that you brought it up, we should also have a tabIndex and onKeyDown added to the Tabs for keyboard navigation... I updated one of the Tags in the sandbox example with this.

This sandbox example shows how we use it... https://codesandbox.io/s/actionable-tag-icon-does-not-change-3fpff

The Carbon tag page https://www.carbondesignsystem.com/components/tag/usage has a non-working example of pretty much what we are doing.
image

@aagonzales
Copy link
Member

Ok what I'm seeing the sandbox seems ok with me then. I was thinking more of a "selectable" which would be another variant.

@guigueb
Copy link
Contributor Author

guigueb commented Apr 19, 2021

@tw15egan
@emyarod
@aagonzales

I know this is closed and it does address the onClick event.
But it doesn't address any other actionable event - ondblclick, ondrag, onfocus, ...

I suppose it can be faked by adding a no-op onClick event = { onClick={() => {}} } to get the visible changes while doing the other action but it seems kinda silly.

Is there a reason why we choose to only deal with the onClick event?
If not, should I log another enhancement?

@emyarod
Copy link
Member

emyarod commented Apr 19, 2021

can you elaborate on what you mean by addressing other actionable events? they should already be spread into the component and the interactive outline appears on focus and clicks

@guigueb
Copy link
Contributor Author

guigueb commented Apr 20, 2021

The PR handled onClick but not other actionable events like onDoubleClick, onMouseOver, etc
I'm not 100% on the onMouseOver but the onDoubleClick should have the UI feedback like onClick.
Basically, this issue was to handle all actionable events (not 100% sure what they all are - but it only handles onClick).

Either they should be allowed - and all give UI feedback,
or they should be disallowed and not do anything.

// addressed
<Tag type="red" title="Clear Filter" onClick={() => console.log("onClick")}> Red
// not addressed - and there are likely others
<Tag type="magenta" title="Clear Filter" onDoubleClick={() => console.log("onDoubleClick")}> Magenta
<Tag type="purple" title="Clear Filter" onMouseOver={() => console.log("onMouseOver")}> Purple

@emyarod
Copy link
Member

emyarod commented Apr 20, 2021

the PR modified the cursor when the component is interactive, and the component already had focus and hover styles which overlap with onMouseOver, onDoubleClick, etc.

are you suggesting that there should be additional visual feedback on double clicks and mouseovers?

although I am assuming from your code snippet that you are implying that these events are not handled? that should not be the case in the current component, but I will need additional clarification from you about that

image

@guigueb
Copy link
Contributor Author

guigueb commented Apr 20, 2021

Q) are you suggesting that there should be additional visual feedback on double clicks and mouseovers?
A) yes, I'm not 100% on the onMouseOver but the onDoubleClick should have the UI feedback like onClick.

And I'm pretty sure there are other actionable events that need to be considered.

From the PR only onClick flags the interactive tag.
image

This sandbox will show that only onClick is handled.. https://codesandbox.io/s/actionable-tags-qxqyj
While all three have actionable event handlers only the onClick Tag has UI indicating to indicate that.

@emyarod
Copy link
Member

emyarod commented Apr 20, 2021

I'm not sure if double click interactions are part of the design system (cc @aagonzales) but the workaround here of course is to manually add the interactive class. on a general note, we typically do not modify styles based on JS event handlers outside of ones that coincide with the hover, focus, etc. CSS pseudoclasses, and I think it would be unreasonable to include the entire list of JS events here to get the cursor: pointer style rule

on a side note, I am not certain if double click interactions are fully supported by screen readers

@guigueb
Copy link
Contributor Author

guigueb commented Apr 20, 2021

Agreed, which is why I initially suggested an 'isInteractive' property.
As it is, a onClick no-op will work as an 'isInteractive' property, kludgy but it will work :-)

<Tag
      type="magenta"
      title="onDoubleClick hacked with an onClick no-op"
      onDoubleClick={() =>
        console.log("onDoubleClick hacked with an onClick no-op")
      }
      onClick={() => {}} // <<<=====  added just to get the interactive UI settings
>
      onDoubleClick hacked with an onClick no-op
</Tag>

@DragosRistici
Copy link

Double-clicking typically selects a paragraph or line of text and HTML content. You can "double-click" icons of apps to open them, even on web, which is a behavior imported from the desktop apps. Also you could "double-click" to perform cell-based interactions of web tables, but here usually you have 2 actions back-to-back: selecting the cell with the first click and entering edit mode with the second, or selecting the app icon with the first and opening it with the second (On the accessibility part, its Tab for selection and Enter/Return for the action).

I didn't find any submitting actions through buttons that use a hover and also a double-click event on the web. I'm sure there are, but I don't believe that's the norm. Clicking usually means a change in the webpage, either through hyperlinks or submit buttons. Hovering is used to show the responsiveness to the clicking of different elements on the screen: hyperlinks, drop-down menus, buttons, etc.

I think it would be tricky from a user experience point of view to try to override the double click behavior of the browser.

So for clickable tags, if they have the hover/focus/active/etc usual button states, the double-click should be treated just like the regular button treats it. Double-clicking a tag feels like should be a visual feature for a CMS admin in the add/edit/delete tag nomenclator functionality that opens the tagName edit feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: tags proposal: open This request has gone through triaging. We're determining whether we take this on or not. type: bug 🐛
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

7 participants