Display a pointer cursor hovering add variant buttons #2062
Conversation
|
Well, I'd say that the problem is in the CSS rather than the markup. Button can even be thought to be better semantically, because the action that you are about to perform is not a Also, I'd doubt that this is the only place, which uses buttons, so applying such a change site-wide using css should a 1 liner. |
|
@mtomov thanks for your feedback! I initially thought the same thing and I was surprised that buttons do not have |
|
I don't think these elements should have the "pointer" cursor. I think the buttons which do have the pointer cursor are a mistake due to them being "Buttons shouldn’t have a hand cursor" by Adam Silver argues for this and has some reference to authorities saying the same: Microsoft, Apple, w3c. cc @Mandily who may have some thoughts on the matter |
|
I'm of the thought that anything interactive on the page should have the affordance of a pointer cursor, or a text cursor if it's an input. I understand where the author of that linked post is coming from, that in theory interactive things should look like they can be interacted with, and therefore don't need the cursor to tell users. But in practice, we have flat designs with less affordance, and the pointer cursor only helps to reinforce. |
|
I read that article before doing this PR but I don't fully agree with the author. Like a lot of comments do, I also think that it's more important to focus on user expectations. I think that Microsoft and Apple resources linked refer to Desktop application, which maybe is a bit different from a web application. w3c takes a strong position in favor of not using pointer cursors for elements that are not links. Anyway I'm starting to feel that this discussion is too much general and would require a separate issue for discussion. If we decide to go with removing pointer from buttons (and other elements that are not links) we probably have to change a lot of stuff in both backend and frontend markup/style. |
kennyadsl
force-pushed
the
admin_order_hover_add_variant
branch
from
eb7d03a to
b884a26
Compare
|
Updated the PR so that we now have |
| @@ -62,9 +62,9 @@ | |||
| </td> | |||
| <td class="actions"> | |||
| <% if can? :update, shipment %> | |||
| <%= link_to '', '#', class: 'save-method fa fa-check no-text with-tip', | |||
| <%= button_tag '', class: 'save-method fa fa-check no-text with-ti, type: :buttonp', | |||
| data: {'shipment-number' => shipment.number, action: 'save'}, title: Spree.t('actions.save') %> | ||
| <%= link_to '', '#', class: 'cancel-method fa fa-cancel no-text with-tip', | ||
| <%= button_tag '', class: 'cancel-method fa fa-cancel no-text with-ti, type: :buttonp', |
They are not real links so semantically it is better to use button tags instead of anchors.
kennyadsl
force-pushed
the
admin_order_hover_add_variant
branch
from
b884a26 to
ac81e0a
Compare
|
Big fan of changing empty anchors to buttons 👍 |
|
thanks! |
The button tag does not display a pointer cursor. Using an anchor tag also uniforms the markup with other similar buttons present in the line items management area.
Before
After