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

EuiFormRow Label Missing Focus Style #826

Closed
cqliu1 opened this issue May 11, 2018 · 5 comments
Closed

EuiFormRow Label Missing Focus Style #826

cqliu1 opened this issue May 11, 2018 · 5 comments
Labels

Comments

@cqliu1
Copy link
Contributor

cqliu1 commented May 11, 2018

I passed the same ID to EuiFormRow and the input control, and when I click on the EuiFormRow label, it focuses the input, but the label doesn't change to blue. It looks like the EuiFormLabel isn't receiving the focus class when the input field is focused. It's probably due to all the wrapper components in between the EuiFormRow and the form control.

<div class="euiFormRow" id="repeatImage-1-size-0-row">
    <label class="euiFormLabel" for="repeatImage-1-size-0">
        <span class="canvas__arg--label">Image size</span>
    </label>
    <div class="canvas__arg--simple">
        <div class="canvas__arg--simple-controls">
            <div class="render_to_dom">
                <div class="euiFlexGroup euiFlexGroup--gutterSmall euiFlexGroup--responsive">
                    <div class="euiFlexItem">
                        <div class="euiFormControlLayout">
                            <input type="number" id="repeatImage-1-size-0" value="100" class="euiFieldNumber">
                        </div>
                    </div>
                </div>
            </div>
        </div>
        <div class="canvas__arg--remove">
            <button class="euiButtonIcon euiButtonIcon--danger" type="button" aria-label="Remove">
                <svg class="euiIcon euiIcon--medium euiButtonIcon__icon" aria-hidden="true" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink"
                    width="16" height="16" viewBox="0 0 16 16">
                    <defs>
                        <path id="trash-a" d="M11 3h5v1H0V3h5V1a1 1 0 0 1 1-1h4a1 1 0 0 1 1 1v2zm-7.056 8H7v1H4.1l.392 2.519c.042.269.254.458.493.458h6.03c.239 0 .451-.189.493-.458l1.498-9.576H14l-1.504 9.73c-.116.747-.74 1.304-1.481 1.304h-6.03c-.741 0-1.365-.557-1.481-1.304l-1.511-9.73H9V5.95H3.157L3.476 8H8v1H3.632l.312 2zM6 3h4V1H6v2z"></path>
                    </defs>
                    <use xmlns:xlink="http://www.w3.org/1999/xlink" xlink:href="#trash-a"></use>
                </svg>
            </button>
        </div>
    </div>
</div>
@snide
Copy link
Contributor

snide commented May 11, 2018

This looks like the same issue @w33ble is discussing here #823

I put this together if you want to manually build out your labels without the magic. It'll allow you to control the focus state on your own.

https://codesandbox.io/s/8yz3npzool

@snide
Copy link
Contributor

snide commented May 14, 2018

Need to chat with @chandlerprall later about how we want to handle this stuff. Essentially EuiFormRow does a lot of clone magic to apply all the focus / id states. But it only works with inputs that are direct children. We probably want to do something more flexible where you can assign a target. Not sure how we'd do that, and if we can't, we should at least provide some docs on how to build form rows manually for custom components like this. One problem we have at the moment is that EuiFormRow provides some presentation CSS as well, so my provided example above isn't a great fix. We can always abstract the styling into it's own component, but my guess is we can do something more clever for this.

@snide
Copy link
Contributor

snide commented May 18, 2018

I realize I gave bad advice on this one. There is in fact an onFocus and onBlur prop passed down to children of EuiFormRow you can use. This will turn the coloring on and off for the label.

In combonation with id you can construct your custom inputs how you need.

I'll post an example in a bit.

@cqliu1
Copy link
Contributor Author

cqliu1 commented Jun 5, 2018

so this is how the react code looks

          <EuiFormRow
            label={
              <Tooltip text={help} placement="left">
                <span className="canvas__arg--label">{label}</span>
              </Tooltip>
            }
            id={argId}
          >
            {simpleArg}
          </EuiFormRow>

I was playing around with the component, and I found out adding a <span> or <div> wrapper around the children of the EuiFormRow component gave me the correct label coloring on focus.

          <EuiFormRow
            label={
              <Tooltip text={help} placement="left">
                <span className="canvas__arg--label">{label}</span>
              </Tooltip>
            }
            id={argId}
          >
            <div>{simpleArg}</div>
          </EuiFormRow>

@snide
Copy link
Contributor

snide commented Jun 6, 2018

EuiFormRow has some functions you can call to manage focus and blur. I'll need to write some docs on constructing custom components, but you can see it in a simple example in the new color picker I'm writing.

https://github.com/snide/eui/blob/b51ad357b6edecbcab7675ed32dbd223227af2a5/src/components/color_picker/color_picker.js#L25-L41

Essentially you do a check in your custom component to see if the the prop (this.props.onFocus) is passed down from EuiFormRow, which itself clones and applies these props to its direct children. If so, you can use it however you need in your inner component and it will pass the proper coloring states back up to the labels. You normally want to do a check for it though as I'm doing there, because your custom component might not be used in combo for EuiFormRow.

https://github.com/elastic/eui/blob/master/src/components/form/form_row/form_row.js#L130-L136

I'll close this issue once I get some better documentation up that describes the above in the EUI docs directly, but that's definitely the way you should be managing focus with anything custom.

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

No branches or pull requests

2 participants