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

Bye bye Qtip2 and hello Hint.css Tooltips #1596

Merged
merged 70 commits into from
Jun 22, 2017
Merged

Conversation

DevinWalker
Copy link
Member

@DevinWalker DevinWalker commented Mar 29, 2017

Description

Fixed conflict from: #1376

Resolves #619 and submitted specifically to release/1.9

Does the following:

  • Removed qtips from being enqueued anywhere
  • Removes qtips from bower, and any pre-processing tools
  • Removes qtips from tests
  • Removes qtips markup throughout the plugin
  • Adds hint.css to be enqueued via Admin and frontend
  • Adds hint.css markup throughout the plugin
  • No strings were changed at all.

There's room for improving this implementation of hint.css to give users more flexibility, but this PR is just about replacing qtips with hint.css out of the box. Additional features can come later.

I tested each individual tooltip in the Admin and front-end. If we merge this, these changes will have to be done in Add-ons as well.

Items check for compatibility

  • give_add_qtip() in wp-admin
  • Backwards compatibility with old tooltips
  • Multiple popular themes
  • Multiple donation form display modes
  • Multiple browsers
  • Multiple devices

Screenshots (jpeg or gifs if applicable):

Types of changes

  • Code improvement
  • Compatibility improvement
  • Performance improvement

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows has proper inline documentation.

mathetos and others added 20 commits January 4, 2017 18:30
…athetos-release/1.9

# Conflicts:
#	assets/css/give-admin-rtl.css
#	assets/css/give-admin-rtl.min.css
#	assets/css/give-admin.css
#	assets/css/give-admin.min.css
#	assets/js/admin/admin-scripts.min.js
#	assets/js/frontend/give.all.min.js
#	assets/sourcemaps/give-admin.css.map
#	assets/sourcemaps/give.css.map
#	includes/forms/template.php
#	templates/give-rtl.css
#	templates/give-rtl.min.css
#	templates/give.css
#	templates/give.min.css
@DevinWalker DevinWalker self-assigned this Mar 29, 2017
Devin Walker added 4 commits March 29, 2017 17:10
* release/1.8.6:
  Thou shalt not support PHP 5.2
  Improved payment status language
  Reordered to fix travis error
  Attempting to fix broken PHPunit tests
  Fix: typo in currency section setting
  Fix: generate level class and id with price_id instead of
  fix #1592
  Update README.md
  Update Documentation.
  Text update
  Removed duplicate
  returns array of objects
  Return “all” rather than empty to prevent conflicts with level ID 0
  Remove space
  Prettier nonce failure notice #1571
  Code reformatting
  Removed unused variables
  Removed unused code #1571

# Conflicts:
#	assets/js/admin/admin-scripts.min.js
* master: (36 commits)
  New POT file for 1.8.6
  Gulp
  Additional adjustments to responsive handling #1544
  Typo fix
  Basic fix for responsive dropdown #1544
  Versions updated and readme added for 1.8.6
  Removing during tests
  Still testing
  Remove codesniffer for now
  Updating composer.json to fix 7.1 error
  New .gitattributes file export ignore file - brought over from Woo New PHPCS ruleset - brought over from Woo
  Fix: stop firing additional ajax request while choosing form for exporting donors
  Fix: php notices which comes when requesting for form levels by ajax
  Update: run gulp concat_scripts
  Add: global js to toggle terms
  Remove: inline toggle term and conditions js
  Fix: improve code formatting
  Fix: php compatibility releated issue of code
  Update:  should be an array
  Refacter: multi donation form payment test
  ...

# Conflicts:
#	assets/css/give-admin-rtl.min.css
#	assets/css/give-admin.min.css
#	assets/js/admin/admin-scripts.min.js
#	assets/js/frontend/give.all.min.js
#	assets/sourcemaps/give-admin.css.map
@DevinWalker DevinWalker changed the base branch from release/1.9 to release/2.0 April 14, 2017 04:41
@DevinWalker DevinWalker reopened this Apr 14, 2017
@ravinderk
Copy link
Collaborator

ravinderk commented Apr 28, 2017

@DevinWalker I reviewed this PR and find following issues:

1. The tooltip content max width is not set.

before

screen shot 2017-04-28 at 9 42 21 am

after

screen shot 2017-04-28 at 9 26 58 am

2 . The tooltip is not showing for the wrong amount.

before

screen shot 2017-04-28 at 9 47 46 am

after

screen shot 2017-04-28 at 9 48 12 am

3. The tooltip is not mobile friendly.

before

screen shot 2017-04-28 at 10 07 32 am

after

screen shot 2017-04-28 at 10 07 57 am

4. The tooltip is not working on the donation listing page.

before

screen shot 2017-04-28 at 11 37 11 am
screen shot 2017-04-28 at 11 45 42 am

after

screen shot 2017-04-28 at 11 37 20 am
screen shot 2017-04-28 at 11 45 55 am

5. The tooltip is not working on the donation detail page.

before

screen shot 2017-04-28 at 11 42 05 am

after

screen shot 2017-04-28 at 11 41 58 am

6. The tooltip is not working on the logs page.

before

screen shot 2017-04-28 at 11 44 34 am

after

screen shot 2017-04-28 at 11 44 54 am
screen shot 2017-04-28 at 11 47 29 am

7. The tooltip is not working on the reports page.

before

screen shot 2017-04-28 at 11 48 19 am

after

screen shot 2017-04-28 at 11 48 27 am

8. The tooltip is not working on the system info page.

before

screen shot 2017-04-28 at 9 24 02 pm

#### after

screen shot 2017-04-28 at 9 24 09 pm

Note To resolve point 3 we can look this example https://osvaldas.info/elegant-css-and-jquery-tooltip-responsive-mobile-friendly

@DevinWalker
Copy link
Member Author

Requirements for Merging

1. Tooltips are backwards compatible with the current qtip2 ones. - [ ]

Perhaps this can be completed by detecting the current dom class/attributes with qtips and initializing hint.css rather.

2. Remove the usage of the word "fake" throughout the PR - [ ]

I understand we're simply extending hint.css so it can be prefixed with simply "Give"

@DevinWalker DevinWalker removed their assignment Jun 13, 2017
@ravinderk
Copy link
Collaborator

@DevinWalker Both point fixed.
For example:
I tested Qtip backward compatibility with https://github.com/WordImpress/Give-Form-Field-Manager.
Here are some screenshots.
screen shot 2017-06-13 at 11 54 37 am
screen shot 2017-06-13 at 11 54 43 am
screen shot 2017-06-13 at 11 54 51 am
screen shot 2017-06-13 at 11 54 56 am
screen shot 2017-06-13 at 11 55 01 am
screen shot 2017-06-13 at 11 55 06 am

But hint.css can face some problem with overflow:hidden property.
You can see some of the tooltip hidden within div but we can fix this problem by changing their position.

for ref: chinchang/hint.css#78
For example:
Before
screen shot 2017-06-13 at 11 54 56 am

After
screen shot 2017-06-13 at 12 23 01 pm

Copy link
Collaborator

@ravinderk ravinderk left a comment

Choose a reason for hiding this comment

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

@DevinWalker This PR is working fine for me

@DevinWalker DevinWalker merged commit 20bf7d2 into release/2.0 Jun 22, 2017
@DevinWalker DevinWalker deleted the mathetos-release/1.9 branch June 25, 2017 09:22
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

3 participants