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

Allow devs to add 'no-link' class to elements to prevent order view link being triggered on row click #18708

Merged
merged 3 commits into from Jan 31, 2018

Conversation

Projects
None yet
6 participants
@mikejolley
Copy link
Member

mikejolley commented Jan 31, 2018

Closes #18704

@claudiulodro

This comment has been minimized.

Copy link
Contributor

claudiulodro commented Jan 31, 2018

Works great. Just needs merge conflicts resolved.

@rodrigoprimo rodrigoprimo self-requested a review Jan 31, 2018

@rodrigoprimo
Copy link
Contributor

rodrigoprimo left a comment

I can confirm that if the class no-link is added to the <tr> element the order view link is not triggered on row click but the cursor is still set to pointer instead of default. It seems to me that this is happening because of the following CSS rule that is applied to the <td> elements:

cursor: pointer;

@mikejolley

This comment has been minimized.

Copy link
Member Author

mikejolley commented Jan 31, 2018

@rodrigoprimo how did you add it to the TR? The intention was for this to be added to new elements within the cells.

@rodrigoprimo

This comment has been minimized.

Copy link
Contributor

rodrigoprimo commented Jan 31, 2018

@mikejolley I used the following code (not production ready as it is missing an if statement to add the class only in the order list page):

add_filter( 'post_class', function( $classes ) {
	$classes[] = 'no-link';
	return $classes;
} );

The intention was for this to be added to new elements within the cells.

I thought the intention was to allow devs to prevent order view link being triggered on the whole row and not just on new elements and that is why I tested adding the class to the <tr>.

@claudiulodro claudiulodro merged commit 664aa28 into master Jan 31, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@claudiulodro claudiulodro deleted the fix/18704 branch Jan 31, 2018

@jranavas

This comment has been minimized.

Copy link

jranavas commented Feb 1, 2018

Hello. Can't "no-link" be the default approach? This is the UX default WordPress/WooCommerce tables operation, prevents unwanted clicks and permits text selections, as @webdados described: #18704
If is not possible, how we can add this fix?
Thanks.

@mikejolley

This comment has been minimized.

Copy link
Member Author

mikejolley commented Feb 1, 2018

The preview modal can be copy pasted. That has the important stuff.

@rodrigoprimo

This comment has been minimized.

Copy link
Contributor

rodrigoprimo commented Feb 2, 2018

If is not possible, how we can add this fix?

You can add the no-link class to the <tr> or to specific <td> to prevent order view link being triggered on click.

@webdados

This comment has been minimized.

Copy link
Contributor

webdados commented Feb 2, 2018

You can add the no-link class to the or to specific to prevent order view link being triggered on click.

This is great. That way we can target only some TD or the entire TR.
(Although I'm not a fan of the original change of turning a TR element clickable.)

@jranavas

This comment has been minimized.

Copy link

jranavas commented Feb 3, 2018

I found the way to add the "no-link", maybe is useful to others. I hope this is the right way, thanks!

/**
 * Add "no-link" class to tr's from WooCommerce orders screen
 * Link: https://github.com/woocommerce/woocommerce/pull/18708
 * Hook reference: https://developer.wordpress.org/reference/hooks/post_class/
 * Tested with: WooCommerce 3.3.1-rc.1
 */
function add_no_link_to_post_class( $classes ) {
	if ( current_user_can( 'manage_woocommerce' ) ) { //make sure we are shop managers 
        foreach ( $classes as $class ) {
	        if( $class == 'type-shop_order' ) {
	            $classes[] = 'no-link';
	        }
    	}
    }
    return $classes;
}
add_filter( 'post_class', 'add_no_link_to_post_class' );
@webdados

This comment has been minimized.

Copy link
Contributor

webdados commented Feb 4, 2018

I don't think you need to check for "manage_woocommerce".

add_filter( 'post_class', function( $classes ) {
	if ( is_admin() ) {
		$current_screen = get_current_screen();
		if ( $current_screen->base == 'edit' && $current_screen->post_type == 'shop_order' ) $classes[] = 'no-link';
	}
	return $classes;
} );
@esaulfarfan

This comment has been minimized.

Copy link

esaulfarfan commented Oct 18, 2018

Thanks @jranavas :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment