-
Notifications
You must be signed in to change notification settings - Fork 59
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
Improve user experience #49
Improve user experience #49
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nossa velho, obrigadão pela ajuda, fazia um tempo que estava procurando algo para substituir esse script e ter uma experiencia melhor em mobile.
Fiz alguns comentários de coisas que precisam ser arrumadas para funcionar bem pra todo mundo.
Se não tiver tempo de alterar essas coisas vai estar tudo bem, posso arrumar depois de fazer merge e de boa.
Obrigado novamente.
@@ -72,11 +72,11 @@ public function load_order_data( $data ) { | |||
public function enqueue_scripts() { | |||
$suffix = defined( 'SCRIPT_DEBUG' ) && SCRIPT_DEBUG ? '' : '.min'; | |||
|
|||
wp_register_script( 'jquery-maskedinput', plugins_url( 'assets/js/jquery-maskedinput/jquery.maskedinput' . $suffix . '.js', plugin_dir_path( __FILE__ ) ), array( 'jquery' ), '1.4.1', true ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Talvez seria melhor manter registrando jquery-maskedinput
mesmo chamando o script novo, já que não a implementação é a mesma, já que alguns outros plugins e projetos usam o script que vem deste plugin e trocar isso provavelmente iria gerar problema para umas pessoas.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
O ruim é que vai carregar os dois scripts no DOM, deixando a página mais pesada. A interface dos dois plugins é a mesma: .mask
. A diferença é que o maskedinput
usa 9
para dígitos obrigatórios e o jquery.mask
usa 9
para dígitos opcionais (no caso de máscaras numéricas), e isso vai dar conflito carregando os dois scripts.
O que sugere nesse caso?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No caso poderia carregar duas vezes o nomes script, porque poderia carregar o jQuery.mask, mesmo chamando ele de jquery-maskedinput
. E a galera teria que atualizar para carregar apenas um.
Ou chamar logo apenas de jquery-maskedinput
para o novo script. Também é uma opção, não bonita, mas que não vai gerar problemas.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minha preocupação em relação a isso é que pode quebrar a máscara de quem configurou seguindo a documentação do maskedinput
, como o caso que falei do dígito obrigatório/opcional.
Aqui vai parar de funcionar, pois a máscara deveria ser 000.000.000-00
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Então deixa quieto isso, melhor atualizar e notificar a galera sobre a isso xD
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Depois que mergear esse eu abro um no pagseguro corrigindo a máscara lá :)
includes/class-wc-ecfb-front-end.php
Outdated
); | ||
|
||
$new_fields['billing_rg'] = array( | ||
'label' => __( 'RG', 'woocommerce-extra-checkout-fields-for-brazil' ), | ||
'placeholder' => _x( 'RG', 'placeholder', 'woocommerce-extra-checkout-fields-for-brazil' ), | ||
'class' => array( 'form-row-last', 'person-type-field' ), | ||
'required' => false | ||
'required' => false, | ||
'type' => 'tel' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RG é coisa de doido, tem uns que tem letras também, além de não existe formatação que funcione para todos os estados (por isso não tem máscara também).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opa, nunca tinha visto com letra, haha, vou reverter essa parte 👍
includes/class-wc-ecfb-front-end.php
Outdated
@@ -236,6 +242,7 @@ public function checkout_billing_fields( $fields ) { | |||
if ( isset( $fields['billing_postcode'] ) ) { | |||
$new_fields['billing_postcode'] = $fields['billing_postcode']; | |||
$new_fields['billing_postcode']['class'] = array( 'form-row-last', 'address-field' ); | |||
$new_fields['billing_postcode']['type'] = 'tel'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Se usar isso com o CEP vai deixar de funcionar corretamente para quem vende dentro e fora do Brasil, o plugin suporta uma opção para escolher os campos de CPF/CNPJ se não for venda no Brasil, e ai tem vários lugares que usam letras no postcode
, exemplos: Canadá e Inglaterra.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mas esse campo hoje tem máscara, permitindo apenas números, certo? Ou perdi algo nesse setup do plugin? PS: sou super noob em wordpress
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tem máscara, mas se eu me lembro bem, alguém me fez carregar a máscara apenas se o pais é o Brasil ou eu tenho que fazer isso ainda xD
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tem razão:
$( document.body ).on( 'change', '#billing_country', function() {
if ( 'BR' === $( this ).val() ) {
wc_ecfb_frontend.maskBilling();
} else {
wc_ecfb_frontend.unmaskBilling();
}
});
Vou fazer um commit para melhorar isso 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dava para aproveitar o script ai e trocar o tipo de campo do CEP, de text
para tel
se for no Brasil, acho que não seria um problema em mobile.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fiz isso, aproveitei e fiz o mesmo em todos que estavam fazendo masking/unMasking, deixando com tipo tel
fixo apenas cpf/cnpj: ea7473e
@claudiosanches tem previsão de quando isso virará release? Ta complicado usar no mobile :( |
@thiagogsr até este final de semana, estava tentando mexer com isso, mas com estava correndo com esse lance de mudar o WooCommerce 2.7 para 3.0. |
@thiagogsr valeu cara, muito obrigado. |
Melhora a experiência do usuário com as melhoras:
0000-0000
e00000-0000
, ao invés de0000-00000
como está atualmente