Skip to content
This repository has been archived by the owner on Dec 12, 2024. It is now read-only.

Modal size - Closes #1700 #1703

Merged
merged 4 commits into from
Aug 8, 2016
Merged

Modal size - Closes #1700 #1703

merged 4 commits into from
Aug 8, 2016

Conversation

diegoeis
Copy link
Contributor

Before, we need to use two classes to change the modal size:
ls-modal-box and ls-modal-large to make big modal or ls-modal-box ls-modal-small to make small modal. Now, you can use only ls-modal-large or ls-modal-small, and the old way works too.

TODO: we need to change where we put this classes. The right place is add a classe ls-modal-large|small in the element ls-modal, because this element is the parent of modal.

This Closes #1700

diegoeis added 3 commits July 15, 2016 16:33
the margin-left was pushing modal to left.
…#1700

Before, we need to use two classes to change the modal size:
`ls-modal-box and ls-modal-large` to make big modal, ou `ls-modal-box
ls-modal-small` to make small modal. Now, you can use only
`ls-modal-large` or `ls-modal-small`, and the old way works too.

TODO: we need to change where we put this classes. The right place is
add a classe ls-modal-large|small in the element ls-modal, because this
element is the parent of modal.

This Closes #1700
@itumoraes
Copy link
Contributor

Acho que a implementação está ok, mas uma coisa me incomoda.

Da forma anterior, quando era utilizado ls-modal-box junto com ls-modal-large, a leitura fazia sentido pois eu tinha uma classe para controlar o box e outra para controlar o tamanho.

Quando assumimos que a utilização passa a ser somente ls-modal-small, ls-modal-large ou ls-modal-box, na minha opinião, fica um pouco esquisito simplesmente retirar essa classe box e compreender que substituir ela por small ou large, faz com que os estilos se preservem.

Fiquei imaginando se não seria mais bacana fazer uma ls-modal-small-box, ls-modal-default-box e ls-modal-large-box, mantendo as outras por questão de retrocompatibilidade e retirando no futuro. Assim a leitura passaria a fazer mais sentido.

@diegoeis
Copy link
Contributor Author

diegoeis commented Aug 5, 2016

@itumoraes Eu já acho que não faz sentido do jeito que está feito hoje, nem do jeito que foi feito originalmente.

A classe .ls-modal-small não devia ficar junto do .ls-modal-box, que só controla a caixa de conteúdo, devia estar na classe .ls-modal, que é a que controla a Modal.
Queria ter feito desse jeito, só não fiz por causa de retrocompatibilidade.

@itumoraes
Copy link
Contributor

@diegoeis Concordo. Vamos pensar nessa evolução futuramente.

@itumoraes itumoraes added this to the 3.8.6 milestone Aug 8, 2016
@itumoraes itumoraes merged commit aca1bc9 into master Aug 8, 2016
@deividmarques deividmarques deleted the modal-size branch October 7, 2016 16:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants