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

[Bug]: Native RadioTile validation sub-optimal #10343

Closed
2 tasks done
brunnerh opened this issue Jan 5, 2022 · 8 comments · Fixed by #16237
Closed
2 tasks done

[Bug]: Native RadioTile validation sub-optimal #10343

brunnerh opened this issue Jan 5, 2022 · 8 comments · Fixed by #16237
Assignees

Comments

@brunnerh
Copy link

brunnerh commented Jan 5, 2022

Package

carbon-components, carbon-components-react

Browser

Chrome

Package version

v10.50.0

React version

v7.50.0

Description

Currently there are two issues when making a RadioTile selection required with native form validation:

  • The required attribute has to be specified on every RadioTile (in React at least) instead of on the TileGroup
  • The invisible underlying input is not positioned correctly to display validation errors at a suitable location.

image

The radio should probably be at the bottom middle of the tile or the group as a whole like this:

image

Additions for this example:

.bx--tile-group { position: relative }
.bx--tile-input {
  bottom: 0;
  left: 50%;
}

CodeSandbox example

https://codesandbox.io/p/sandbox/required-tile-group-q33x2q

import React from "react";
import { render } from "react-dom";
import { Button, Form, TileGroup, RadioTile } from "carbon-components-react";

function onSubmit(e) {
  e.preventDefault();
  alert("submitted");
}

const App = () => (
  <div style={{ padding: "2em" }}>
    <Form onSubmit={onSubmit}>
      <TileGroup name="radio">
        <RadioTile required>Tile 1</RadioTile>
        <RadioTile required>Tile 2</RadioTile>
        <RadioTile required>Tile 3</RadioTile>
      </TileGroup>
      <Button type="submit">Submit</Button>
    </Form>
  </div>
);

render(<App />, document.getElementById("root"));

Steps to reproduce

  • Add TileGroup with required RadioTiles to a form.
  • Submit form without selecting any of the tiles.

Code of Conduct

@brunnerh
Copy link
Author

As I found out, validation messages can also be top-aligned (Safari). So if this is implemented, that should be taken into consideration.

@jnm2377 jnm2377 added severity: 3 https://ibm.biz/carbon-severity type: bug 🐛 and removed type: enhancement 💡 labels Feb 2, 2022
@tay1orjones tay1orjones moved this to ⏱ Backlog in Design System Dec 12, 2022
@tay1orjones
Copy link
Member

The invisible underlying input is not positioned correctly to display validation errors at a suitable location.

I think this part has been fixed, the other issue though remains to my knowledge

@2nikhiltom
Copy link
Contributor

Hey @brunnerh

The invisible underlying input is not positioned correctly to display validation errors at a suitable location.

Above issue has been fixed by PR

I can't reproduce the other issue, and I didn't encounter any errors popup in the CodeSandbox link provided with the bug report.
grp-tile

Also, based on the screenshots provided in the bug(Attached above this as well), it seems that the observed behavior is related to RadioButtonGroup and RadioButton, rather than TileGroup and RadioTile.
i.e
CodeSandbox-Link

<FormGroup>
        <RadioButtonGroup name="group" legendText="Storage tier (disk)">
          <RadioButton required labelText="Free (1 GB)" value="free" />
          <RadioButton required labelText="Standard (10 GB)" value="standard" />
          <RadioButton required labelText="Pro (128 GB)" value="pro" />
        </RadioButtonGroup>
</FormGroup>

Could you confirm this or further help with a CodeSandbox that reproduces this issue with TileGroup and RadioTile. Thanks :)

@brunnerh
Copy link
Author

This is about TileGroup, and the problem still exists (at least in v10).
The sandbox broke due to not linking to a fixed version of the packages.

I updated the link.

@brunnerh
Copy link
Author

brunnerh commented Apr 10, 2024

In v11, native validation is completely broken in the React package, because required is not set/forwarded.

<input
checked={checked}
className={`${prefix}--tile-input`}
disabled={disabled}
id={inputId}
name={name}
onChange={!disabled ? handleOnChange : undefined}
onKeyDown={!disabled ? handleOnKeyDown : undefined}
tabIndex={!disabled ? tabIndex : undefined}
type="radio"
value={value}
ref={ref}
/>

StackBlitz

@2nikhiltom
Copy link
Contributor

Hey @brunnerh Thanks for quick response, we are looking into this.

@2nikhiltom 2nikhiltom moved this from ⏱ Backlog to 🏗 In Progress in Design System Apr 17, 2024
@2nikhiltom 2nikhiltom moved this from 🏗 In Progress to 🚦 In Review in Design System Apr 22, 2024
@github-project-automation github-project-automation bot moved this from 🚦 In Review to ✅ Done in Design System Apr 25, 2024
@brunnerh
Copy link
Author

@2nikhiltom
This issue is not fully fixed.
The positioning of the validation message is still not so great, in some browsers it completely obscures the tile contents.

@2nikhiltom
Copy link
Contributor

@brunnerh
This majorly added support for required in RadioTile
For positioning in other browsers could you reproduce the issue in a stackblitz and open a new bug to track ?

kennylam added a commit to kennylam/carbon that referenced this issue Jul 30, 2024
…on-design-system#10343)

* feat(dropdown): syncing dropdown react v11
* feat(combo-box): syncing with react v11
---------

Co-authored-by: kennylam <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

6 participants