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

Update massautocomplete.js #36

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

MartyIX
Copy link

@MartyIX MartyIX commented Oct 15, 2015

This makes CSS styling of input box a bit easier.

I'm not sure if it isn't too specific use-case. But it would help me a lot. :)

This makes CSS styling of input box a bit easier. 

I'm not sure if it isn't too specific use-case. But it would help me a lot. :)
@hakib
Copy link
Owner

hakib commented Oct 15, 2015

Can you give an example of when you would want to style a closed ac-container (rather then having it hidden)?

@MartyIX
Copy link
Author

MartyIX commented Oct 15, 2015

I'm not styling ac-container. I'm styling the search <input> based on container state:

image

image

Note the corners of the <input>.

@hakib
Copy link
Owner

hakib commented Oct 15, 2015

My main concern is that if we descide to implement it this way it wont work as exepcted right out of the box if you descide not to use the theme.css - You would have to manually add the display:none rule to your own css.

How about a solution a long the lines of this example.

@MartyIX
Copy link
Author

MartyIX commented Oct 15, 2015

Well, my implementation provides extra possibilities to style input but I wouldn't force the look to all users. If anybody would want the same look as above, then he/she can do that. That's really it. I mean if your concern is not performance, then it's a not a breaking change.

Your implementation does not behave as good as mine. I can see the input without rounded corners sometime when there should be rounded corners.

@MartyIX
Copy link
Author

MartyIX commented Nov 2, 2015

Is there a chance that you merge this? As I stated before this is not a BC break, I just added a new class so a user has more possibilities and not less.

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 this pull request may close these issues.

2 participants