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

Action: No way to set href for links #9138

Open
2 of 10 tasks
geospatialem opened this issue Apr 16, 2024 · 8 comments
Open
2 of 10 tasks

Action: No way to set href for links #9138

geospatialem opened this issue Apr 16, 2024 · 8 comments
Labels
0 - new New issues that need assignment. ArcGIS Map Viewer Issues logged by ArcGIS Map Viewer team members. bug Bug reports for broken functionality. Issues should include a reproduction of the bug. estimate - 3 A day or two of work, likely requires updates to tests. p - medium Issue is non core or affecting less that 60% of people using the library

Comments

@geospatialem
Copy link
Member

Check existing issues

Actual Behavior

It is not currently possible to have an action be a link.

Expected Behavior

Expecting the same behavior as button (and eventually split-button's primary button via #9126) where I can pass an href and the inner button element will become a styled link and the props get reflected properly.

Should include the following native props:

  • href
  • target
  • rel
  • download

Reproduction Sample

https://codepen.io/geospatialem/pen/JjVZgNL

Reproduction Steps

  1. Open the Codepen
  2. Observe the workflow which requires JS using the open method

Reproduction Version

2.7.1

Relevant Info

No response

Regression?

No response

Priority impact

p3 - want for upcoming milestone

Impact

No response

Calcite package

  • @esri/calcite-components
  • @esri/calcite-components-angular
  • @esri/calcite-components-react
  • @esri/calcite-design-tokens
  • @esri/eslint-plugin-calcite-components

Esri team

ArcGIS Map Viewer

@geospatialem geospatialem added bug Bug reports for broken functionality. Issues should include a reproduction of the bug. 0 - new New issues that need assignment. p - medium Issue is non core or affecting less that 60% of people using the library ArcGIS Map Viewer Issues logged by ArcGIS Map Viewer team members. estimate - 3 A day or two of work, likely requires updates to tests. needs milestone Planning workflow - pending milestone assignment, has priority and/or estimate. labels Apr 16, 2024
@geospatialem geospatialem changed the title Action: No way to set href for links on the primary button Action: No way to set href for links Apr 16, 2024
@driskull
Copy link
Member

I think we should discuss this one a bit first. Since the action component is used within action-bar (toolbar), I'm not sure it would make sense to support some of the actions being links.

In this case, why can't they use a calcite-button?

@driskull
Copy link
Member

Some of the concerns i have

  • Potential issues with a11y due to roles of parent components consuming actions and expecting a button role.
  • Unexpected behavior from an end user when clicking an action and not expecting a URL to change or be opened in a new window.

Can we research the accessibility issues and look into the UX a bit? I could see it being problematic when we are stying both a button and a link to look exactly the same with no visual difference to an end user. Its also weird for a component's role to change dynamically whether its a link or not. Maybe these should be separate components in order to avoid the confusion and potential a11y issues.

@geospatialem
Copy link
Member Author

In this case, why can't they use a calcite-button?

In the use case, teams were using tile prior to its redesign. We recommended they slot in an action, but it required them to also add in JS to support a link: https://codepen.io/geospatialem/pen/JjVZgNL. A button doesn't have the same meaning in the use case. cc-ing @macandcheese if there is a different pattern we should recommend here though.

image
image

Some of the concerns i have

  • Potential issues with a11y due to roles of parent components consuming actions and expecting a button role.
  • Unexpected behavior from an end user when clicking an action and not expecting a URL to change or be opened in a new window.

Makes sense on the URL opening, think this speaks to having the dev add in a meaningful label that depicts a link is present and will send the user to either a new window or tab. We could add some doc around this support.

Can we research the accessibility issues and look into the UX a bit? I could see it being problematic when we are stying both a button and a link to look exactly the same with no visual difference to an end user. Its also weird for a component's role to change dynamically whether its a link or not. Maybe these should be separate components in order to avoid the confusion and potential a11y issues.

Believe the action component is currently using a button role, so maybe we should strategize if that's still the correct approach, and how to proceed, if at all, with the above request.

@macandcheese
Copy link
Contributor

macandcheese commented Apr 24, 2024

I don't see an issue with allowing Action to conditionally render as a anchor - it's the same in Button currently. Ultimately the onus of making the UX make sense and actions apparent to a user falls to the implementer - so the same as any of our current components.

There are definitely places where using an Action inline in this way is preferable to a Button for UI - internally the Button renders an anchor or button element, so seems reasonable to have Action behave the same way. Ultimately it's a different UI expectation than Button.

@driskull
Copy link
Member

I don't see an issue with allowing Action to conditionally render as a anchor - it's the same in Button currently. Ultimately the onus of making the UX make sense and actions apparent to a user falls to the implementer - so the same as any of our current components.

It's not exactly the same as button. Button doesn't have any parent components like action-bar that will soon expect a toolbar role with buttons inside. If we allow action to be a link this could cause issues with accessibility and expected roles. We would have to specify in documentation not to do this. This kind of doc leads me to believe separate components might make more sense here. Like an action-link?

I think we should spike and discuss this. Specifically if components should change role internally depending on a property value (anchor link vs button). A separate discussion involving design should be made to whether links and buttons can or should look exactly the same as that could be confusing to an end user because there would be no way to visually tell them apart.

@macandcheese
Copy link
Contributor

macandcheese commented Apr 24, 2024

Yeah, we should verify those use cases and it might come down to doc.

From the design side - we already have the same set up in Link and Button - each renders either an a or button depending on the presence of the same properties. It will always come down to the implementer to determine the correct UI / UX to communicate to and inform a user, combined with correctly populated a11y / aria.

@driskull
Copy link
Member

From the design side - we already have the same set up in Link and Button - each renders either an a or button depending on the presence of the same properties. It will always come down to the implementer to determine the correct UI / UX to communicate to and inform a user, combined with correctly populated a11y / aria.

Having a button render a link or a link render a button has always seemed off to me. I just wanted to bring it to get more attention. We will just need to document correct usage.

@macandcheese
Copy link
Contributor

Yeah, totally fair. Link especially as that one insinuates href by name, IMO.

For Button, it's pretty common to render either a / button based on href, looking at other systems: https://mui.com/material-ui/react-button/#contained-button, https://ant.design/components/button, https://developer.microsoft.com/en-us/fluentui#/controls/web/button, etc.

That said, some like Atlassian have (newly introduced) distinct components for Icon Button, Link Button, Split Button, etc.

@geospatialem geospatialem added this to the 2025-01-28 - Jan Release milestone Aug 7, 2024
@geospatialem geospatialem removed the needs milestone Planning workflow - pending milestone assignment, has priority and/or estimate. label Aug 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0 - new New issues that need assignment. ArcGIS Map Viewer Issues logged by ArcGIS Map Viewer team members. bug Bug reports for broken functionality. Issues should include a reproduction of the bug. estimate - 3 A day or two of work, likely requires updates to tests. p - medium Issue is non core or affecting less that 60% of people using the library
Projects
None yet
Development

No branches or pull requests

3 participants