Skip to content
This repository was archived by the owner on Oct 2, 2019. It is now read-only.

allow-clear with theme=bootstrap broken in 0.9.6 #578

Closed
zavidovych opened this issue Jan 13, 2015 · 28 comments
Closed

allow-clear with theme=bootstrap broken in 0.9.6 #578

zavidovych opened this issue Jan 13, 2015 · 28 comments

Comments

@zavidovych
Copy link

http://plnkr.co/edit/yzoAL0PRkOyGHeuMbGj2?p=preview

unkjfgzpb8

@artkoenig
Copy link

It does not relate to allow-clear, the bootstrap-layout is broken completely:

http://plnkr.co/edit/2mQ6S7w01mjKhWUMEf6b?p=preview

It was ok in 0.9.2

@jack-kerouac
Copy link

👍

1 similar comment
@gerardp
Copy link

gerardp commented Jan 20, 2015

👍

@BP18
Copy link
Contributor

BP18 commented Jan 20, 2015

👍

The Demo Plunker linked to from the README.md should be updated to use the latest version of select.css and select.js. Then this issue is immediately obvious there.

  1. Temp workaround for text alignment (probably a better way to fix this):
.ui-select-bootstrap > .ui-select-match > .ui-select-toggle {
    text-align: left;
}
  1. Temp workaround for missing bootstrap styles (box-shadow, etc.):
    Add css class form-control to button tag on line 2 of match.tpl.html

@abobwhite
Copy link

@BP18 Thanks. I just tried adding your two workaround adjustments for version 0.9.6 and it didn't appear to make any difference in the styling.

@abobwhite
Copy link

It appears that if the allowClear directive is included, there is a conditional ui-select-allow-clear CSS class added that does nothing (no usage in select.js or select.css)...

This is very frustrating.

@BP18
Copy link
Contributor

BP18 commented Jan 22, 2015

Hi @abobwhite, I'm not using allow-clear, but my workarounds seem to work on the demo page.

Here's the Demo Plunker as-is other than upgrading to 0.9.6 so I can reproduce the problem:
http://plnkr.co/edit/TEt1lqLYSwHo30EQdLvr?p=preview

Here's the same Plunker but with my two fixes added:
http://plnkr.co/edit/M1A5BhAK7et987Bhgih3?p=preview

The padding is still messed up, but at least the text-align and other styles are better. Don't think this is really a proper fix, but hopefully it helps figure out what's going on.

@abobwhite
Copy link

@BP18 Thanks.

I'm confused - isn't this issue surrounding the allow-clear styling? allow-clear on both plunkr's yield the same styling issue. If you weren't talking about allow-clear, what styling were you trying to fix, exactly?

@BP18
Copy link
Contributor

BP18 commented Jan 22, 2015

@abobwhite, in my first demo Plunker link with just the upgrade to 0.9.6, the Bootstrap theme control is messed up. Without focus on the control, the text is center aligned and the field does not have the other Bootstrap form-control css styles like box-shadow and transition: border-color ease-in-out.

@artkoenig
Copy link

The template did change from

bildschirmfoto 2015-01-22 um 21 41 53

to

bildschirmfoto 2015-01-22 um 21 42 40

could this be the cause?

@BP18
Copy link
Contributor

BP18 commented Jan 22, 2015

@artkoenig, yes I think that is where it broke. Commit 6e94894.

Previously the button tag had the form-control css class applied, but with the new template it doesn't. This caused the shadow, transition, and other styles to be lost. The text-align and padding were being controlled by overrides in the select.css file, but that broke with this template change too.

@BP18
Copy link
Contributor

BP18 commented Jan 23, 2015

Slightly tweaked my original fixes and submitted a pull request.

@gzsombor
Copy link

I tried to test your changeset, but the 'allow-clear' button is still broken. Is it supposed to be fixed by your changeset ?

@abobwhite
Copy link

That was my understanding as well - it sounds like there are two issues here. Broken bootstrap styles in general as well as broken allow-clear styling for bootstrap. I am most concerned about the allow clear but I'm sure both need to be addressed.

@BP18
Copy link
Contributor

BP18 commented Jan 23, 2015

Yes, I think @abobwhite is correct about there being two (related?) issues here. I'm not using allow-clear and neither does the demo page, so my fix didn't address that part.

@BP18
Copy link
Contributor

BP18 commented Jan 23, 2015

I played around with allow-clear=true and I see the second problem you are mentioning (the X/clear button appearing below the field instead of integrated). I think my pull request addresses the other styling issues and doesn't make the allow-clear issue worse, so hopefully it can be part of a complete solution.

Also, there was a recent pull request (#486) that was supposed to add allow-clear support to the bootstrap template, but it doesn't work properly.

@starf
Copy link

starf commented Feb 3, 2015

Thanks for all the hard work already put in, a proper fix would be awesome.
tl;dr +1

@tarlepp
Copy link

tarlepp commented Feb 11, 2015

+1 for this

@tarlepp
Copy link

tarlepp commented Feb 11, 2015

Meanwhile you can use this little CSS trick:

.ui-select-clear {
    position: relative;
    float: right;
    top: -26px;
    right: 25px;
    border: none;
    background: none;
    margin-bottom: -26px;
}

@dimirc
Copy link
Contributor

dimirc commented Feb 16, 2015

With #606 merged, can you confirm if all works ok now?

@JeromeLam
Copy link
Contributor

@dimirc That merge doesn't seem to fix the problem. Still looks like the screen grab from the original post.

@dimirc
Copy link
Contributor

dimirc commented Feb 16, 2015

@JeromeLam you are right, seams that the problem is the way allow-clear was implemented at #486

I'm checking the code and maybe reverting that PR and do a different implementation

@JeromeLam
Copy link
Contributor

@dimirc Looking at #486, I see the template for the clear button uses btn btn-default. Using btn-link will at least get rid of the border box. It may then just be a question of styling it using part of the CSS trick from tarlepp 5 days ago.

@dimirc
Copy link
Contributor

dimirc commented Feb 16, 2015

@JeromeLam based on @bampakoa comment I pushed PR #667

Check updated plunker and I think now should be ok

@JeromeLam
Copy link
Contributor

@dimirc It's definitely looking better now! I suggest we add btn-link to the class list for the clear link so there is no odd outline and box shadow when the control is active or in focus (at least on Chrome).

I've created PR #669 tor this and forked your plunker with the change applied.

@gzsombor
Copy link

Have you checked in firefox ? It seems that the clear button is still below the drop down, however now, it's on the right side.

@starf
Copy link

starf commented Feb 16, 2015

FYI: That happens only when the input is so long it overlaps the X button.

@dimirc
Copy link
Contributor

dimirc commented Feb 16, 2015

Should be solved with #667

@dimirc dimirc closed this as completed Feb 16, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests