Page MenuHomePhabricator

Special:Nearby refresh button does not change cursor
Closed, ResolvedPublic

Description

When you hover the Refresh button in the top right of Special:Nearby, the cursor does not change as it normally would when hovering a button or link.

Acceptance criteria

  • A cursor should show (as a pointer) when you hover over Nearby refresh icon

developer notes

You could achieve this by adding href="#" to the nearby refresh link so that it automatically shows a cursor pointer OR switch it to a <button> and add a css rule for #secondary-button elements

Event Timeline

TheDJ raised the priority of this task from to Needs Triage.
TheDJ updated the task description. (Show Details)
TheDJ added a project: MobileFrontend.
TheDJ added a subscriber: TheDJ.

For clarification it's not oojs ui it's just HTML and a tiny bit of css.

ah right. i thought i saw ooui classnames there, but it's mw-ui. We probably have better button buttons definitions for MobileFrontend somewhere (the notifications icon that it is actually replacing in this view is probably a better implementation already). PS is that notifications button supposed to be replaced, or is that accidental ?

bd808 triaged this task as Low priority.
bd808 added a subscriber: bd808.
Jdlrobson updated the task description. (Show Details)
Jdlrobson added a subscriber: Volker_E.

@Jdlrobson I wonder what lead to the decision not to markup the element as mw-ui-button mw-ui-quiet mw-ui-icon mw-ui-icon-element. The reason I can see is more padding, that might have been circumvented by leaving out in order to bring it on one line with the hamburger.
Which on the other hand is not a good idea in my opinion as the clickable area is now at 24px of height. IMO the big question here is, before adding a pointer cursor just to a.mw-ui-icon-elements, are we (and more important our users) satisfied with the clickable area of the hamburger icon?
I'd probably open up another task on that question.

@Jdlrobson I wonder what lead to the decision not to markup the element as mw-ui-button mw-ui-quiet mw-ui-icon mw-ui-icon-element. The reason I can see is more padding, that might have been circumvented by leaving out in order to bring it on one line with the hamburger.
Which on the other hand is not a good idea in my opinion as the clickable area is now at 24px of height. IMO the big question here is, before adding a pointer cursor just to a.mw-ui-icon-elements, are we (and more important our users) satisfied with the clickable area of the hamburger icon?
I'd probably open up another task on that question.

I personally believe the solution is to add these classes to all our icons that are clickable including the hamburger.

Jdlrobson raised the priority of this task from Low to Medium.Aug 17 2017, 3:39 PM
Jdlrobson updated the task description. (Show Details)

Change 372492 had a related patch set uploaded (by D3r1ck01; owner: Alangi Derick):
[mediawiki/extensions/MobileFrontend@master] Add href to refresh icon on Special:nearby to change cursor

https://gerrit.wikimedia.org/r/372492

Change 372492 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Add href to refresh icon on Special:Nearby to change cursor

https://gerrit.wikimedia.org/r/372492

Confirmed as fixed! Thanks @D3r1ck01 !

You are very much welcome anytime. Always ready to fix more issues and make the wiki world a better place @Jdlrobson 😊.

It's better to add an empty href, then one with an anchor in cases like these. Both should change the cursor, but # will scroll you to the top of the page, and might get registered on the navigation stack when actually reached.
But, at the very least, when you take this approach, you have to change the click handler function, to preventDefault() the click event, so that it will not try to actually attempt to follow the link.

And for future reference, an alternative fix for this problem would have been to force the cursor with CSS and to set role=button to tell screenreaders that the <a> element was navigable. It could be argued that this is a 'better' approach, as there is no actual page navigation happening there, so 'button' is better than 'link' in this case. It could even be argued that this shouldn't be a <a> element to begin with, but a button element, but i'm not sure if that is used anywhere else in the current UI, so that might be more problematic to implement.

@TheDJ, thanks so much for the suggestion. All these options are okay by me and it's true about the preventDefault(), in your opinion, between the empty href and applying CSS on that <a> tag, which one is a better solution. If we agree on a particular solution, I will do a patch. Due to this, I will reopening this ticket and tagging @Jdlrobson to hear from him too :).

I should note on the nearby page changing the anchor is not a big deal (the icon is already at the top of the page), but yes you can fix this for completeness.

@Jdlrobson, should having an href with no # do the job? Or should we go by the CSS way?

Up to you @D3r1ck01 I think the closing paragraph of https://phabricator.wikimedia.org/T126387#3536889 should guide you best!

Up to you @D3r1ck01 I think the closing paragraph of https://phabricator.wikimedia.org/T126387#3536889 should guide you best!

Great :)

Change 372869 had a related patch set uploaded (by D3r1ck01; owner: Alangi Derick):
[mediawiki/extensions/MobileFrontend@master] Change Special:Nearby refresh Icon to pointer on-hover

https://gerrit.wikimedia.org/r/372869

@TheDJ, @Jdlrobson, now we have another patch that uses the button tag and then CSS that changes the cursor to pointer on hover :).

Change 372869 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Change Special:Nearby refresh Icon to pointer on-hover

https://gerrit.wikimedia.org/r/372869