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

fix: prevent tooltip span from wrapping input element #3499

Closed
mathetos opened this issue Jul 17, 2018 · 4 comments

Comments

Projects
None yet
3 participants
@mathetos
Copy link
Member

commented Jul 17, 2018

User Story

As a Give User I expect to be able to style the checkboxes in my Give forms in a consistent manner.

Current Behavior

I currently struggle with the "Create an account" checkbox because it resides within the <span> of the tooltip contrary to the Terms & Conditions checkbox and other checkboxes which correctly are NOT wrapped in a <span>.

Expected Behavior

I expect that similar form inputs throughout the Give form would have identical markup to ensure simple and consistent styling with minimal CSS.

Possible Solution

It's not clear to me yet why this function results in the <span> wrapping the <input> element but this is where that happens:
https://github.com/WordImpress/Give/blob/release/2.2/includes/forms/template.php#L1288-L1296

It seems this is actually intentional in order to trigger a tooltip directly on the checkbox, but (1) it's just not awesome markup; (2) it creates style problems. See Visuals below for an example.

I think in order to show that Registration is "required", we should instead customize the (?) tooltip to indicate that, since that is visually where donors will look for clarifying information. There's nothing to indicate that they should look for additional information while hovering over the checkbox -- plus you have to actually click the checkbox on mobile to trigger that tooltip, so overall it's not a good experience.

Additionally, I don't believe the hint.css requires that the element be a <span>, we can just use the <i> to trigger the tooltip.

CURRENT MARKUP

<label for="give-create-account-578">
					<span aria-label="Registration is required to donate." class="hint--top hint--bounce" rel="tooltip">
						<input
							type="checkbox" name="give_create_account" value="on" id="give-create-account-578"
							class="give-input give-disabled" checked="">
					</span> <span>Create an account</span>
					<span class="give-tooltip hint--top hint--medium hint--bounce"
					      aria-label="Create an account on the site to see and manage donation history."
					      rel="tooltip"><i class="give-icon give-icon-question"></i></span> </label>

SUGGESTED MARKUP

Ideally a simple fix to that function will result in this markup:

<label for="give-create-account-578">
                    <input type="checkbox" name="give_create_account" value="on" id="give-create-account-578" class="give-input give-disabled" checked="">Create an account
                    <i class="give-icon give-icon-question give-tooltip hint--top hint--medium hint--bounce" aria-label="Create an account on the site to see and manage donation history." rel="tooltip"></i>
</label>

Visuals

image

Acceptance Criteria

  • The markup is consistent with other checkboxes in Give form, similar to the example above.
  • The tooltip still works as intended.
  • The markup is consistent in all variations of whether the form requires registration or is optional.

@kevinwhoffman kevinwhoffman changed the title fix(form) Prevent tooltip span from wrapping input element fix: prevent tooltip span from wrapping input element Oct 10, 2018

@kevinwhoffman

This comment has been minimized.

Copy link
Member

commented Oct 10, 2018

@kakshak Please move forward with Matt's suggested markup and ensure the acceptance criteria are met in doing so.

@kakshak

This comment has been minimized.

Copy link
Member

commented Oct 11, 2018

Slack Call Summary

Participants: @kakshak, @ravinderk
Topic: Update HTML markup as per Matt suggestion
Result: I'll look over it and if hint.css will break/affect to other functionality into the plugin then leave that for tooltip only and provide an update to Kevin for that.

@kakshak

This comment has been minimized.

Copy link
Member

commented Oct 11, 2018

@kevinwhoffman I checked the suggested markup for tool-tip and here is the scenario for that.

  • In current markup <input> is not wrapped into <span>. So, that is already updated.
  • The old markup for the tooltip is designed globally. To make the changes as per Matt's suggestion, we need to change lot the HTML, CSS and JS and this change will take a lot of CSS makeover which is functional globally as of now. So, It will be good if we kept the html as it is that is now only for tooltip. I've discussed regarding this with Ravinder.

Below is the new/current markeup

<label for="give-create-account-10">
	<input type="checkbox" name="give_create_account" value="on" id="give-create-account-10" class="give-input">Create an account
	<span class="give-tooltip hint--top hint--medium hint--bounce" aria-label="Create an account on the site to see and manage donation history." rel="tooltip">
		<i class="give-icon give-icon-question"></i>
	</span>
</label>

Please let me know your thoughts on this.

@kevinwhoffman

This comment has been minimized.

Copy link
Member

commented Oct 11, 2018

I have reviewed the current markup in Give v2.3.0 and confirmed the checkbox is not wrapped within the tooltip span. Leave the tooltips markup as is. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.