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

Inline edit snippet page - clear/empty "x" icons #1076

Closed
mcoker opened this issue Jun 14, 2018 · 12 comments · May be fixed by #1119
Closed

Inline edit snippet page - clear/empty "x" icons #1076

mcoker opened this issue Jun 14, 2018 · 12 comments · May be fixed by #1119
Assignees

Comments

@mcoker
Copy link
Contributor

mcoker commented Jun 14, 2018

  • I noticed that since edge has a built-in "x" icon to clear/empty an <input type="text"> element, the "x" icon we use in PF to clear the input value creates 2 "x"s. Screenshot below.

screen shot 2018-06-14 at 1 20 47 pm

  • Also the "combobox" implementation has an "x" icon to clear the current value, but the design spec does not. I understand that it may be because the combobox in the design is currently empty, so an "x" icon to clear it wouldn't make sense. Assuming including the "x" in the snippet implementation is correct, I'll just mention that I think having 2 "x" icons that look as similar as they do when editing the combobox is a bit confusing. Screenshot below.

screen shot 2018-06-14 at 1 38 36 pm

@LHinson
Copy link
Member

LHinson commented Jul 6, 2018

@michael-coker thanks for filing this. We have a number of issues on our plate but will take a look at this next sprint.

@mcarrano
Copy link
Member

mcarrano commented Jul 9, 2018

Yeah, this bothers me a little bit, too, @michael-coker . The second "X" icon is actually a cancel button so it has a different result but I can see where this can be confusing. I believe that the clear "X" is being exposed by the component itself.

Do you have any thoughts on how to address this? We definitely need the cancel button, but could consider a different icon.

@mcoker
Copy link
Contributor Author

mcoker commented Jul 9, 2018

@mcarrano just to make sure, we're talking about the 2 x's in the combobox element? Assuming so, how about we just drop the 1st x (the one that clears the field) from a combobox? The clear button is only on the text input currently, and isn't on any other type of form element. So I don't see an issue with dropping it from the combobox element.

And FWIW, that clear button only shows up before the first time it's clicked. If you edit the combobox, click the "x" to clear it, put in some text, lose focus on the element, then re-focus, you'll notice the "x" to clear the field isn't there. I'm assuming that's a bug?

Re: the duplicate clear x's in IE/Edge, we could probably just use browser prefixes to exclude them from those browsers.

@mcarrano
Copy link
Member

mcarrano commented Jul 9, 2018

I agree with this solution.

@mcoker
Copy link
Contributor Author

mcoker commented Jul 9, 2018

@mcarrano cool! I can grab this one if you like, but it may be a couple of weeks before I can get to it.

@LHinson
Copy link
Member

LHinson commented Jul 9, 2018

@michael-coker that would be great, I'll assign you to it but leave it in the "to-do" column for now

@mcoker
Copy link
Contributor Author

mcoker commented Aug 2, 2018

@mcarrano finally getting back around to this. I don't think I understood what you meant when when you said the first/clear "X" in the combobox was part of the component itself. But you're right, it's part of the combobox component, and doesn't look like you can remove it, so I'll just leave that one in there. Does that sound OK?

@mcarrano
Copy link
Member

mcarrano commented Aug 2, 2018

@michael-coker I think so, but can you show me an example of what that would look like? In re-reading this now I have to admit to being a bit confused about what's going on here and what we want to accomplish. Unfortunately having the Clear and Cancel buttons at the same time causes issues. Open to alternate suggestions.

@mcoker
Copy link
Contributor Author

mcoker commented Aug 2, 2018

Basically, I would make no changes to the existing combobox inline edit pattern. I was originally saying that this instance with (2) X's is confusing, and I was hoping to just get rid of the first/clear X, but it's built into the combobox component (an external resource), and there are no options to remove it.

screen shot 2018-08-02 at 9 46 13 am

Though it is a bit confusing. I'm not sure how to handle it differently. We should probably avoid trying to re-style the combobox, and it isn't a simple CSS tweak to drop the clear/X button from that component. And the only way I can think of changing our pattern so those buttons aren't confusing is to reposition or restyle our check/X buttons, but I don't know how to do that while keeping those buttons consistent with the rest of the form elements/buttons in the inline-edit pattern.

mcoker added a commit to mcoker/patternfly-3 that referenced this issue Aug 2, 2018
@mcoker
Copy link
Contributor Author

mcoker commented Aug 3, 2018

@mcarrano just pinging to make sure you saw my reply above (I forgot to tag you)

@mcarrano
Copy link
Member

mcarrano commented Aug 3, 2018

@michael-coker That seems like a good resolution. Should we just close this out then?

@mcoker
Copy link
Contributor Author

mcoker commented Aug 3, 2018

@mcarrano there is still the issue with 2 X clear icons in the text input field in IE. Submitted a PR for that - patternfly/patternfly#1119

@LHinson LHinson closed this as completed Aug 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants