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

THROBBER! #173

Merged
merged 4 commits into from Apr 23, 2015
Merged

THROBBER! #173

merged 4 commits into from Apr 23, 2015

Conversation

soniktrooth
Copy link
Contributor

Bringing back the throbbing functionality on autocomplete text fields.

@soniktrooth
Copy link
Contributor Author

To test this you can just add an entity reference field to a content type.

@pirog
Copy link
Contributor

pirog commented Apr 22, 2015

@soniktrooth
The only thing that i can see here is we might just want to set it up so people can't toggle off fontawesome anymore. If they did that presumably this would break?

@soniktrooth
Copy link
Contributor Author

It wouldn't so much break as just not have a throbber–as is the case ATM. I'm happy to run with FA requirement as we discussed the other day though.

@pirog
Copy link
Contributor

pirog commented Apr 22, 2015

right.

so either we

  1. use the default drupal throbber when FA is not on
  2. just always make sure FA is on

the choice is yours. we can do it in this PR or merge this in and you can open a new issue for this thing.

@soniktrooth
Copy link
Contributor Author

I like the idea of getting the Drupal one if there's no FA. Ideas for passing this knowledge through the css? Maybe add a body class when FA is added?

@pirog
Copy link
Contributor

pirog commented Apr 22, 2015

despite my loathing of the normal throbber i agree that this is probably the best thing to do.

could preprocess the field(s) that have the throbber and add the FA or drupal class based on the theme setting? Not sure if that would work in this case but it would be pretty easy.

@soniktrooth
Copy link
Contributor Author

Ah, it's a theme setting that is taking care of adding FA. That's good—sounds like the best approach. I share your disdain for normal throbber but something is better than nothing when it comes to UX.

I will add some code to this so don't merge yet.

@pirog
Copy link
Contributor

pirog commented Apr 23, 2015

i await all the code

…ave default Drupal throbbing action when it's not.
@soniktrooth
Copy link
Contributor Author

I've said the word 'throbber' far too many times this week. @pirog wanna check this out now?

@pirog
Copy link
Contributor

pirog commented Apr 23, 2015

haha seriously right? yeah this looks good to me!

pirog added a commit that referenced this pull request Apr 23, 2015
@pirog pirog merged commit 3dd659e into drupalprojects:7.x-3.x Apr 23, 2015
@soniktrooth soniktrooth deleted the throbber branch April 23, 2015 19:36
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.

None yet

2 participants