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

Feat/link #388

Merged
merged 4 commits into from
Jul 28, 2022
Merged

Feat/link #388

merged 4 commits into from
Jul 28, 2022

Conversation

phelpa
Copy link
Contributor

@phelpa phelpa commented Jul 22, 2022

image

https://www.figma.com/file/WJWEph4jRV7uuVe4e9Pj13/Desktop-components-handoff?node-id=3023%3A62055

import Link from '@eduzz/houston-ui/Link';

<Link href='https://houston.com' showIcon>
  Houston 
</Link>

Link props

prop tipo obrigatório padrão descrição
children React.ReactNode true -
as React.ElementType false a
href string false -
showIcon boolean false -
size sm, md, inherit false inherit

src/pages/ui-components/Link/index.tsx Outdated Show resolved Hide resolved
src/pages/ui-components/Link/index.tsx Outdated Show resolved Hide resolved
src/pages/ui-components/Link/index.mdx Outdated Show resolved Hide resolved
src/pages/ui-components/Link/index.tsx Outdated Show resolved Hide resolved
@ffernandomoraes ffernandomoraes added the enhancement New feature or request label Jul 22, 2022
@ffernandomoraes
Copy link
Member

Concordo com os pontos do Miguel (infelizmente hehe)

@ffernandomoraes
Copy link
Member

Fala meu monstro!

Analisando melhor, acho que esse componente deveria ser um sub-componente do Typography por se tratar de um componente de texto.

Outro ponto é que eu não acho válido ele ter tamanhos. Em 99% dos casos ele deverá ser utilizado dentro de um <Typography /> que já dita o tamanho dos textos, ele deveria apenas seguir o tamanho do componente pai. Consequentemente mudaria a importação:

  1. import Link from '@eduzz/houston-ui/Typography/Link
  2. import Typography from '@eduzz/houston-ui/Typography e a implementação seria: Typography.Link.

Faz sentido?!

@ffernandomoraes
Copy link
Member

Conversando o @miguelaugl , até faz sentido existir a prop size, mas por padrão deixe ela como: inherit, pra ela respeitar o pai.

@phelpa
Copy link
Contributor Author

phelpa commented Jul 25, 2022

Fala meu monstro!

Analisando melhor, acho que esse componente deveria ser um sub-componente do Typography por se tratar de um componente de texto.

Outro ponto é que eu não acho válido ele ter tamanhos. Em 99% dos casos ele deverá ser utilizado dentro de um <Typography /> que já dita o tamanho dos textos, ele deveria apenas seguir o tamanho do componente pai. Consequentemente mudaria a importação:

  1. import Link from '@eduzz/houston-ui/Typography/Link
  2. import Typography from '@eduzz/houston-ui/Typography e a implementação seria: Typography.Link.

Faz sentido?!

De primeiro momento não faz sentido ter tamanho, já que vamos sempre usar dentro do Typography e ele somente vai acompanhar o tamanho. Mas tem o ícone e ele tem dois tamanhos : sm e md, hoje está sendo setado o tamanho do ícone pela prop size.

Sobre ele ser parte do Typography eu não sei, eu acho que ele tem suas particularidades, como focus, hover e ícone, acho que ele ganhou o direito de ser um componente a parte. O antd e chakra também não misturam o Link com algo relacionado a texto.

@phelpa
Copy link
Contributor Author

phelpa commented Jul 25, 2022

Conversando o @miguelaugl , até faz sentido existir a prop size, mas por padrão deixe ela como: inherit, pra ela respeitar o pai.

Atualizado

@ffernandomoraes
Copy link
Member

Fala meu monstro!
Analisando melhor, acho que esse componente deveria ser um sub-componente do Typography por se tratar de um componente de texto.
Outro ponto é que eu não acho válido ele ter tamanhos. Em 99% dos casos ele deverá ser utilizado dentro de um <Typography /> que já dita o tamanho dos textos, ele deveria apenas seguir o tamanho do componente pai. Consequentemente mudaria a importação:

  1. import Link from '@eduzz/houston-ui/Typography/Link
  2. import Typography from '@eduzz/houston-ui/Typography e a implementação seria: Typography.Link.

Faz sentido?!

De primeiro momento não faz sentido ter tamanho, já que vamos sempre usar dentro do Typography e ele somente vai acompanhar o tamanho. Mas tem o ícone e ele tem dois tamanhos : sm e md, hoje está sendo setado o tamanho do ícone pela prop size.

Sobre ele ser parte do Typography eu não sei, eu acho que ele tem suas particularidades, como focus, hover e ícone, acho que ele ganhou o direito de ser um componente a parte. O antd e chakra também não misturam o Link com algo relacionado a texto.

O Antd usa dentro da Tipografia:
image

E o Chakra UI e o Mui isolaram por se tratar de um componente de "navegação".

Mas a decisão é sua @phelpa , fique a vontade de deixar como você quer. Os dois jeitos são válidos, só muda o POV.

@ffernandomoraes ffernandomoraes merged commit f461f5d into develop Jul 28, 2022
@danieloprado danieloprado deleted the feat/Link branch March 31, 2023 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants