In Umami theme login link not showing underline on hover. All other links are underlined on hover e.g: the main menu, footer links are underlined on hover.

Umami login link no underline on hover

CommentFileSizeAuthor
#9 03-with-patch-applied-hovered.png7.93 KBmarkconroy
#9 02-current-hovered.png7.83 KBmarkconroy
#9 01-default-state.png7.2 KBmarkconroy
#7 2971546-7.patch579 bytesMukeysh
#2 after-applying-patch.png960.69 KBMukeysh
#2 before-patch.png960.7 KBMukeysh
#2 2971546-2.patch577 bytesMukeysh
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Mukeysh created an issue. See original summary.

Mukeysh’s picture

Mukeysh CreditAttribution: Mukeysh as a volunteer and at gai Technologies Pvt Ltd for gai Technologies Pvt Ltd commented
FileSize
2971546-2.patch577 bytes
before-patch.png960.7 KB
after-applying-patch.png960.69 KB

Applying the patch for the above. Please review. Attaching screenshot for reference.

Mukeysh’s picture

Mukeysh CreditAttribution: Mukeysh as a volunteer and at gai Technologies Pvt Ltd for gai Technologies Pvt Ltd commented
Status: Active » Needs review
msankhala’s picture

msankhala CreditAttribution: msankhala as a volunteer and at Srijan | A Material+ Company commented
Issue summary: View changes
msankhala’s picture

msankhala CreditAttribution: msankhala as a volunteer and at Srijan | A Material+ Company commented
Issue tags: +Needs frontend framework manager review 
+++ b/core/profiles/demo_umami/themes/umami/css/components/navigation/menu-account/menu-account.css
@@ -33,3 +33,6 @@
+.menu-account__link:hover {
+  border-bottom: 1px solid;
+}

Using border-bottom for the underline effect to an anchor tag is not a correct way to do it. text-decoration: underline; is more better way to do it.

Umami theme already has these lines of code in menu-account.css

.menu-account__link,
.menu-account__link:hover {
  color: inherit;
  text-decoration: none;
  background-color: inherit;
}

which you are overriding. Looks like it is intentional to not show underline on hover on login link. Need input from core committer or umami theme maintainer whether we want to show hover effect on login link or not.

markconroy’s picture

markconroy CreditAttribution: markconroy at Annertech commented
Status: Needs review » Needs work
Issue tags: -Needs frontend framework manager review

Hi Folks,

Thanks for working on this.

I think we do need something to differentiate hover from normal state, and I think underline is the safest. We have links in the main body underlined when in normal state and not underlined when hovered, so I'd prefer an underline approach rather than a border-bottom one.

If someone can get a patch for that, I'll get it tested.

Mukeysh’s picture

Mukeysh CreditAttribution: Mukeysh as a volunteer and at gai Technologies Pvt Ltd for gai Technologies Pvt Ltd commented
FileSize
2971546-7.patch579 bytes

I have added patch with text-decoration: underline; on hover. Please review.

Mukeysh’s picture

Mukeysh CreditAttribution: Mukeysh as a volunteer and at gai Technologies Pvt Ltd for gai Technologies Pvt Ltd commented
Status: Needs work » Needs review
markconroy’s picture

markconroy CreditAttribution: markconroy as a volunteer and at Annertech commented
Status: Needs review » Reviewed & tested by the community
FileSize
01-default-state.png7.2 KB
02-current-hovered.png7.83 KB
03-with-patch-applied-hovered.png7.93 KB

Thanks @Mukeysh,

Marking this as RTBC.

Just for future reference, we like to provide before/after screenshots for frontend changes, since we don't have automatic regression tools built in for our themes. I'll provide them here.

Current default state:
Umami user links - default

Current state, when hovered:
Umami user links - hovered

Hovered state, after patch applied:
Umami user links - hovered, after patch

Committers, can we also credit @msankhala on this issue please.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed d48980d4c3 to 8.6.x and cd3d2bf6df to 8.5.x. Thanks!

  • alexpott committed b989db1 on 8.6.x 
    Issue #2971546 by Mukeysh, markconroy, msankhala: Umami theme login link...

  • alexpott committed cd3d2bf on 8.5.x 
    Issue #2971546 by Mukeysh, markconroy, msankhala: Umami theme login link...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.