-
Notifications
You must be signed in to change notification settings - Fork 2
Feature/breadcrumb #89
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
base: main
Are you sure you want to change the base?
Conversation
❌ Deploy Preview for egdev6-design-system failed.
|
|
@curiousidy varias cosas: Ajustes visuales: 1- Creo que habría que dxarle menos espaciado entre elementos, quedas muy dispersos 2- en el modo dark, pondría los elemntos intermedios de color blanco por defecto 3- El último elemnto podría usar Text en vez de Link para que no tuviera ningún efecto visual ni de interacción 4-Para los hovered y colores puedes ver como se usan en componentes como input. En vez de negros podrías usar tonos de grises para que estén unificados. En modo light el color rojo es el secondary y en dark es el accent. 5-Le pondría algún efecto de hover cuando abriera elementos JSX 6- Cuando apliques colors asegúrate que aplican también al hover 7- En este componente se me hace rara la sombra roja...si necesitas otra puedes mirar en styles/themes o crearla para que cuadre visualmente 8- Revisa las stories, hay varios errores de accesibilidad Te he sacado bastantes cosas, pero no significa que el trabajo no sea bueno. Este es un componente complejo, lo has encaminado guay. Realiza ajustes y revisamos código en el siguiente PR ;) |
…ces for linter compliance
…rDropdown functionality
…bility and consistency in dark mode
…ance-authority integration for improved styling
… enhanced accessibility and styling adjustments
…tency in Calendar component
…ling and styling consistency
…nge handlers in Calendar stories
…r visual consistency
…t and enhance documentation
…t and enhance size selection feature
…onth in useCalendar hook
…tates in Calendar component, also hover updates
…ng and enhance read-only interactions
…e optimizations, and standardize props for better maintainability
…consistent appearance with out-of-month days
…alendar component documentation
… comparison logic in useCalendar
…te comparisons and state management
… disabled dates handling in useCalendar
…updates and improved disabled date handling
…ar components for improved state management
…story for accurate representation
…nce hover effects
[ADD] ATOM - feat: calendar
|
Buenas @curiousidy ! 1- el default lo dejaría con fondo transparente y sin bordes 2-Revisa que el hover en start y end content aplique a todo, no a los elementos por separado Lo demás lom veo guay!! buen trabajo!! casi lo tenemos! :) |
…-system into feature/breadcrumb
|
Buenas @curiousidy! Un par de cosas 1- Lo que quería decir en el anterior punto 1 es que la idea de los breadcrumb debería ser usarlo en un top page de un view, por lo que tener un fondo de color plano y un efecto hover sobre el container puede hacer que no encaje con el resto del flow de la view. Los hover deberían ser sobre los elementos links individuales del componente, no sobre el container. 2- en los elementos que van con iconos debería comporarse como un uno (encapsula en etiqueta y aplica el hover sobre la etiqueta) |












Buenas tardes Quique,
He implementado los siguientes puntos:
3. Implementación del componente
Desarrollo del componente en base a la definición anterior. Incluir historias de Storybook si procede.
4. Documentación del componente
Ya he hecho la docu y he aplicado las reglas de accesibilidad que habías publicado en linkedin.
Las únicas que no apliqué son:
-"Aria-current"
-"Cada nivel debe ser un enlace accesible, salvo el último"
Creo que esta funcionalidad debería ser del componente link y no del breadcrumb en sí.
Espero tu feedback, seguro hay muchas cosas a mejorar, todo es mejorable 😉.
Gracias.