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

Required checkout fields showing a dotted underline under the required indicator #1262

Open
2 tasks done
mikkamp opened this issue Feb 19, 2020 · 10 comments
Open
2 tasks done
Assignees
Labels
priority: normal The issue/PR is low priority—not many people are affected or there’s a workaround, etc. type: bug The issue is a confirmed bug.

Comments

@mikkamp
Copy link

mikkamp commented Feb 19, 2020

Describe the bug

When viewing the checkout page, the required fields show a red dotted underline under the * character:

image

The element which is used for this text is <abbr>, which some browser style by default with a dotted underline. See: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/abbr

Isolating the problem (mark completed items with an [x]):

  • I have deactivated other plugins and themes and confirmed this bug occurs when only WooCommerce + Storefront theme are active.
  • I can reproduce this bug consistently using the steps below.

To Reproduce

Steps to reproduce the behavior:

  1. Go to https://themes.woocommerce.com/storefront/
  2. Add a product to the cart and proceed to the checkout page
  3. Notice the extra dotted underline on the required fields

Expected behavior

I'd expect this style to be reset so it doesn't show an underline. The CSS for this element is already being altered here: https://github.com/woocommerce/storefront/blob/version/2.5.4/assets/css/woocommerce/woocommerce.scss#L1572-L1575
So it could get an extra line to reset the text-decoration

Browser Environment

It will depend on the Browser which is being used, in my case I'm using Chrome on Ubuntu.

WordPress Environment

Please provide relevant details of your WordPress setup and server environment.

WordPress 5.3.2
WooCommerce 3.9.2
Storefront 2.5.4
@haszari
Copy link
Member

haszari commented Feb 19, 2020

Thanks @mikkamp - reproduced this in 2.5.4 and 2.5.3.

Looks like a simple fix so I've added this to 2.5.5 milestone.

I checked Twenty Twenty and Twenty Nineteen, and both reset abbr to prevent the dotted underline, so I think it's worth doing the same in Storefront.

I wonder if we should consider fixing this upstream in WooCommerce core? cc @peterfabian

@haszari haszari added priority: normal The issue/PR is low priority—not many people are affected or there’s a workaround, etc. type: bug The issue is a confirmed bug. labels Feb 19, 2020
@haszari haszari added this to the 2.5.5 milestone Feb 19, 2020
@haszari haszari self-assigned this Feb 20, 2020
@haszari
Copy link
Member

haszari commented Feb 20, 2020

I checked Twenty Twenty and Twenty Nineteen, and both reset abbr to prevent the dotted underline, so I think it's worth doing the same in Storefront.

Update – I was mistaken, these theme fixes are in WooCommerce core. I'm reporting this issue there, so hopefully it can be fixed for all themes.

I've opened a PR with the fix in Storefront as a fallback – woocommerce/woocommerce#1264.

@haszari
Copy link
Member

haszari commented Feb 20, 2020

This is now logged to WooCommerce core repo: #2156

@haszari
Copy link
Member

haszari commented Feb 20, 2020

Removing this from 2.5.5 – it's not critical or urgent, and wasn't introduced in Storefront 2.5.3.

@haszari haszari removed this from the 2.5.5 milestone Feb 20, 2020
@nielslange
Copy link
Member

@haszari As this issue is related to the WooCommerce core rather than the Storefront theme, can this ticket be closed?

@haszari
Copy link
Member

haszari commented Mar 1, 2020

Closing - to follow / fix this issue please visit #2156

@haszari haszari closed this as completed Mar 1, 2020
@nielslange nielslange reopened this May 4, 2020
@nielslange
Copy link
Member

nielslange commented May 4, 2020

Reopening this issue, as this issue it not caused by WooCommerce but by Storefront. WooCommerce already uses text-decoration: none; for this element in https://github.com/woocommerce/woocommerce/blob/f25967d57bc50c2906c94c111966d1bdbac04d1f/assets/css/woocommerce.scss#L1361-L1367. However, Storefront dequeues the WooCommerce styles in

add_filter( 'woocommerce_enqueue_styles', '__return_empty_array' );
Hence, the fix should be applied within Storefront.

@haszari & @juliaamosova Any thoughts on this or shall I go ahead and close #2156 in benefit of this ticket?

@haszari
Copy link
Member

haszari commented May 4, 2020

Good sleuthing @nielslange ! That's surprising, feels like Woo core and Storefront are working at cross purposes here.

I think this means that this issue could be fixed in either place - and I'm not sure what the best solution is. I'm happy to keep both issues open for now.

@nielslange
Copy link
Member

I think this means that this issue could be fixed in either place - and I'm not sure what the best solution is. I'm happy to keep both issues open for now.

Not really. Technically, the issue is already fixed within WooCommerce and Storefront simply does not show that fix due to

add_filter( 'woocommerce_enqueue_styles', '__return_empty_array' );

Unless we open up Storefont in a way that the WooCommerce styles become active again, this issue needs to be addressed within Storefront. You know what I mean, @haszari? 🙂

@haszari
Copy link
Member

haszari commented May 5, 2020

Unless we open up Storefont in a way that the WooCommerce styles become active again, this issue needs to be addressed within Storefront.

Agreed, just feels unfortunate that we would reimplement a style rule which we get for free but then we've disabled (de-enqueued).

Re-enqueuing the Woo styles (removing the override) is an option, though I'm assuming there's a reason we're doing this and to change now would break other things. (Which is also unfortunate!)

Woo also has a storefront-specific stylesheet where we could presumably fix this, and there are probably other approaches we could take :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: normal The issue/PR is low priority—not many people are affected or there’s a workaround, etc. type: bug The issue is a confirmed bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants