Skip to content
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

Adicionando classe de Post Status #310

Merged
merged 11 commits into from
Jul 12, 2015
Merged

Conversation

matheusgimenez
Copy link
Member

Conforme PR #266 tentei adaptar a classe ao standard do WP.

Usei meta tags para passar os argumentos para o JS, o wp_localize_script não funciona bem com várias instâncias, logo achei melhor fazer assim.

Abraços :)

if( $( document.body ).hasClass( 'post-php' ) || $( document.body ).hasClass( 'post-new-php' ) ) {
console.log('ahoy');
var select = '';
if( typeof args.select !== 'undefined' ) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-1
Aqui dá pra usar só:

if( !args.select ) {

@fdaciuk
Copy link
Contributor

fdaciuk commented Jul 11, 2015

++
Boa @matheusgimenez ✨ 👯

@matheusgimenez
Copy link
Member Author

Opa @fdaciuk, pode me explicar melhor qual o problema do alinhamento? to tentando ajudar mais no odin, porem realmente acabo me enrolando com isso.. mas tô tentando melhorar, hehe

@fdaciuk
Copy link
Contributor

fdaciuk commented Jul 11, 2015

Opa @fdaciuk, pode me explicar melhor qual o problema do alinhamento? to tentando ajudar mais no odin, porem realmente acabo me enrolando com isso.. mas tô tentando melhorar, hehe

Capaz cara, mandou benzaço! :D
O alinhamento é só naquela linha que marquei.. os sinais de = não estão todos corretamente alinhados ;)

*/
$( window ).load( function() {
$( 'meta.odin-custom-status-meta' ).each( function() {
var meta = $( this );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-1
Apoveitando: por convenção, variáveis que contenham elementos do DOM, devem começar com $. Essa variável se chamaria $meta :)

@matheusgimenez
Copy link
Member Author

Acho que resolvi tudo :)

if( $( document.body ).hasClass( 'post-php' ) || $( document.body ).hasClass( 'post-new-php' ) ) {
var select = '';
if( args.select ) {
console.log('ahoy!');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-1
Ficou um console.log perdido aqui ;)

@fdaciuk
Copy link
Contributor

fdaciuk commented Jul 11, 2015

@matheusgimenez, arrumando esses pontos acho que fica legal :)
Depois dá pra fazer um refactory pra separar melhor em funções e fazer um clean code ae 😉

@matheusgimenez
Copy link
Member Author

Bom, removi a manipulação do DOM num dos loops, mas no outro não vejo como.. já que preciso fazer append em cada elemento desse loop.

Sobre o codigo, to tentando, hehe. Valeu pelas dicas. :)

@fdaciuk
Copy link
Contributor

fdaciuk commented Jul 11, 2015

Sem problemas @matheusgimenez, já ficou bem melhor 👯
Depois posso dar uma olhada nisso ;)

Tá tudo funcionando certinho né? Posso fazer o merge?

@matheusgimenez
Copy link
Member Author

Sim, sem problemas :)

fdaciuk added a commit that referenced this pull request Jul 12, 2015
Adicionando classe de Post Status
@fdaciuk fdaciuk merged commit f6d6d7d into wpbrasil:master Jul 12, 2015
@fdaciuk
Copy link
Contributor

fdaciuk commented Jul 12, 2015

Show! Valeu o/

@matheusgimenez
Copy link
Member Author

Galera, criei a documentação para essa classe:

https://github.com/wpbrasil/odin/wiki/Classe-Odin_Post_Status

@fdaciuk
Copy link
Contributor

fdaciuk commented Jul 13, 2015

Boa o/

@rands0n
Copy link

rands0n commented Jul 13, 2015

Boa, só não entendi por que não usou minha branch. Mas tudo bem! Rocks!

Finalmente a feature agora está no odin! 😄

@adammacias adammacias added this to the v2.8.0 milestone Jul 13, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants