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

crear componente icon #22

Merged
merged 8 commits into from
Oct 17, 2023
Merged

Conversation

dam788
Copy link

@dam788 dam788 commented Aug 24, 2023

Link de Tarea:
https://janiscommerce.atlassian.net/browse/JUIP-85

Link de Subatarea:
https://janiscommerce.atlassian.net/browse/JUIP-119

Contexto:
Actualmente no contamos con un componente Icon reutilizable, por lo que nos obliga en todas las apps a tener código repetido, y en caso de querer hacer alguna modificación, se deberá cambiar en las 3 apps por igual.

Necesidad:
Tener el componente Icon reutilizable disponible en el package UI para facilitar su escalabilidad

Props que debe aceptar:

Prop Default Requerido Tipo Obs
name   true string  
color #2979FF false string tomar el color por default de la paleta del package
size 16 false number  

Análisis funcional

  1. Instalar el package react-native-vector-icons
  2. Seguir la instalación para android y ios GitHub - oblador/react-native-vector-icons: Customizable Icons for React Native with support for image source and full styling. : necesitamos el archivo iconmoon.ttf y selection.json. El asset se puede descargar desde:
    - Assets Apps
  3. Crear el componente Icon
  4. Crear muestrario de iconos en Icon.stories.js

Como se puede probar?:

import {Icon} from '@janiscommerce/ui-native';

...
<Icon name="iso_janis" size={60} />

Screenshoots

1 - icons

2 - icons

import iconsSelection from '../../../src/components/Icon/assets/fonts/selection.json';

export default {
title: 'Design system/Icons',
Copy link
Contributor

Choose a reason for hiding this comment

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

Acá habría que agregar:

argTypes: { color: { control: {type: 'color'}, }, },

para que el controlador del color sea un color picker

Screenshot_1692896789

<style>
@font-face {
font-family: 'janisFontIcon';
src: url('../assets/public/janis-font-icon.ttf') format('truetype');
Copy link
Contributor

Choose a reason for hiding this comment

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

Estuve viendo el storybook-web pero no se están viendo los iconos, estuve probando un par de cosas y puede ser que porque cuándo se generan los archivos de .storybook_static, en el iframe.html queda la ubicación de '../assets/public/janis-font-icon.ttf'.

2023-08-24_14-12

Me parece que lo mejor sería agregar la propiedad staticDirs: ['../src/fonts'], dentro del main.js de .storybook, esto hace que cuándo se generen los archivos de storybook-web, también se van a estar agregando las fuentes custom que tengamos en esa carpeta fonts

2023-08-24_13-59

Y también modificar el preview-head.html para que, en vez de estar tomando ../assets/public/janis-font-icon.ttf cómo lo está haciendo, tome ../janis-font-icon.ttf. Igualmente probé y todavía no están apareciendo los iconos en web, pero me parece que por ese lado se podría encarar.

@coveralls
Copy link

coveralls commented Aug 24, 2023

Pull Request Test Coverage Report for Build 6484798199

  • 5 of 5 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%

Totals Coverage Status
Change from base Build 6356436753: 0.0%
Covered Lines: 171
Relevant Lines: 171

💛 - Coveralls

@pablonortiz pablonortiz self-requested a review August 24, 2023 20:33
Copy link
Contributor

@GonzaFran GonzaFran left a comment

Choose a reason for hiding this comment

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

El componente funciona excelente, faltaría pullearse master a la branch para corregir este problema que está pasando cuando tratas de ver los colores desde android. Falta eso nada más y estaríamos.

Peek 2023-08-28 17-26

@dam788 dam788 requested a review from GonzaFran August 30, 2023 13:23
Copy link
Contributor

@christian97dd christian97dd left a comment

Choose a reason for hiding this comment

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

Dami!, estuve probando de linkear delivery con ui native eliminando la fuente de íconos para ver si seguía funcionando. No funcionó :p
image
La semana que viene nos juntamos para ver como podemos hacer, porque estuve probando también un par de alternativas, pero no funcionaron

@dam788 dam788 requested a review from christian97dd October 11, 2023 15:39
@christian97dd christian97dd merged commit e56ab4a into master Oct 17, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants