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

Added awesome complete support. #302

Closed

Conversation

rayrutjes
Copy link
Contributor

Preview:
image

Fixes #298, algolia/github-awesome-autocomplete#34

Let me know what you think.

@Mottie
Copy link
Member

Mottie commented Mar 24, 2016

Hi @rayrutjes!

Sweet thanks! I've gotta run.. if no one gets around to merging this, I'll do it when I get a break later today.

@silverwind
Copy link
Member

Thanks!

  1. Does not work in Firefox for me, because the addon's styles outweigh our styles. Adding !important to every added rule should fix that.
  2. The addon sets .awesome-autocomplete { border: none; }, I'm not sure this is indended but I'd rather not have that border removed:

  1. In your screenshot, there'a a black x icon on the search bar, can you make that brighter?
  2. Your styles seem overly specific. I'd suggest using
.awesome-autocomplete .aa-description

instead of long selectors like

.awesome-autocomplete .tt-dropdown-menu .tt-suggestion .aa-suggestion.aa-repo .aa-description

@Mottie
Copy link
Member

Mottie commented Mar 24, 2016

Also, all instances of #4183C4 need to be replaced with /*[[base-color]]*/ #4183C4 so when the user selects a different base color, it will match.

It also looks like this definition is a tint of the base color... is there a way we could make it match the base color or use a more neutral color?

form#search_form .awesome-autocomplete .twitter-typeahead .tt-input:focus {
  border-color: #51A7E8;
}

@rayrutjes
Copy link
Contributor Author

Ok, working on it, thx

@silverwind
Copy link
Member

is there a way we could make it match the base color or use a more neutral color

One could add filter: brightness(120%) if it's text-only nodes like links.

@Mottie
Copy link
Member

Mottie commented Mar 24, 2016

Oh, one more thing... I set my base-color to green and noticed these issues:

awesome complete

I would try to help, but whenever the page loses focus, the entire dropdown gets emptied, so it is difficult to figure out class names.

@rayrutjes
Copy link
Contributor Author

Regarding the border: none, we will have to make an issue on the autocomplete repo I guess. Is a Firefox only issue.

@silverwind
Copy link
Member

I wonder why it won't show in other browsers. Here is the offending rule.

@rayrutjes
Copy link
Contributor Author

To be sure to not forget anything:

  • Allow for custom color replacements
  • Simplify selectors
  • Make input border visible
  • Test in Firefox and add required "!important"
  • .tt-input:focus neutral color

@rayrutjes
Copy link
Contributor Author

Ok, I am good, now waiting for the border fix on AwesomeComplete's side.

.awesome-autocomplete .aa-company i,
.awesome-autocomplete .aa-description,
.awesome-autocomplete .aa-issue-body{
color: #C0C0C0 !important;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you lowercase the letters in hex colors?

@Mottie
Copy link
Member

Mottie commented Mar 24, 2016

I'm curious... the awesome complete dropdown is showing this repo as having 1230 stars, but the star/unstar button at the top of the page is showing 747. Why is there a huge difference?

And thanks again for all your hard work!

@rayrutjes
Copy link
Contributor Author

Oh my, you are so right, I have no idea about that gap, let's ping Algolia ^^.
I will make the other changes in a few minutes, fixing the border in firefox for now.

@Mottie
Copy link
Member

Mottie commented Mar 24, 2016

I opened an issue: algolia/github-awesome-autocomplete#40

@rayrutjes
Copy link
Contributor Author

The only missing thing is the delete-icon, but my design skills are limited... There is no white version of the icon, do you have a fallback solution? Could we maybe use one of octotree's icon?

@Mottie
Copy link
Member

Mottie commented Mar 24, 2016

I didn't see a delete icon anywhere, if it is solid black, then you can apply an invert filter:

.delete-icon {
  -webkit-filter: invert(100%) !important;
          filter: invert(100%) !important;
}

@rayrutjes
Copy link
Contributor Author

There you go!
image

.awesome-autocomplete .tt-suggestion {
border-color: #343434 !important;
}
.awesome-autocomplete .tt-suggestion .octicon {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you merge this definition with the one above? Everything else looks good to me! Thanks again!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean with the global definitions, those huge lists of classes? 😚

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I get it now, only the octicon?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I meant line 2521-2522.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

He probably means the one on line 2522.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@silverwind thx, it's done!

.awesome-autocomplete .aa-thumbnail ,
.awesome-autocomplete .aa-thumbnail {
background-color: #181818 !important;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the user avatar background? If so, I'd make it slightly brighter to it stands out over the background. See the last user here (not sure why it fails to pull his avatar, he has one):

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, and the star should be grey. If it is SVG, you can do fill: currentColor !important on it.

.awesome-autocomplete .aa-issue-body {
color: #c0c0c0 !important;
}
.awesome-autocomplete .aa-thumbnail ,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unnecessary space before ,

@rayrutjes
Copy link
Contributor Author

Not sure about the thumbnail background, your decision.

@silverwind
Copy link
Member

Well, it would affect any user with transparent backgrounds, which is not GitHub-like. Let's leave it as-is.

@silverwind
Copy link
Member

Merged in 882b634, thanks!

@silverwind silverwind closed this Mar 24, 2016
@rayrutjes rayrutjes deleted the add-awesome-complete-support branch March 24, 2016 21:16
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.

3 participants