Feature/add design system - #1
Conversation
| const { button } = van.tags | ||
|
|
||
| export const Button = ({ text, onClick, type }) => | ||
| button({ onclick: onClick, class: `button ${type || 'default'}` }, text) |
There was a problem hiding this comment.
@Ania-chan Personally, I would add optional parameters to the function for disabled and value attributes, as well as name and an alt description.
There was a problem hiding this comment.
@zr0z I added these params except for value. What do you mean by this? There is already a text passed to this component
There was a problem hiding this comment.
@Ania-chan by default button is assumed to act on a form, so the behavior of the button should be in a value property and it will be sent to the server alongside the name.
If you assume that your component will never be used within a form then it should have a type set to button.
From MDN:
If your buttons are not for submitting form data to a server, be sure to set their type attribute to button. Otherwise they will try to submit form data and to load the (nonexistent) response, possibly destroying the current state of the document.
|
|
||
| const { img } = van.tags | ||
|
|
||
| export const Icon = ({ name, alt = '' }) => img({ class: 'icon', alt, innerHTML: icons[name] }) |
There was a problem hiding this comment.
@Ania-chan The <img> tag should always have a source and it is not supposed to have children nodes.
So you can't use innerHTML here, you could if you were rather using an <object> tag.
With the required src you can display SVG content either by passing the URL of the file, or by using a data URI. You will have to use the javascript function to encode the SVGs before passing it to src.
data:image/svg+xml;charset=UTF-8,[URL encoded SVG code]
The last issue is that icons[name] is actually a promise, so it would needs to be awaited.
For simplicity sake, VanJS is using synchronous component so you would need to use a state to display the icon when it has been awaited.
Finally, as a alt I would personally use ${name} icon and not expose it as a parameter, even better if you add a capitalize function.
| logo: () => import('./icons/Logo.svg') | ||
| } | ||
|
|
||
| export default icons |
There was a problem hiding this comment.
Personally I would export an async function taking the icon name rather than exposing the data structure in which you store the data.
If you decide in the future to change the way you are structuring the data you only have to modify the implementation of that file rather than going through all the other files and changing there.
export default async function (name) {
return icons[name];
}
Related to the issue highlighted above and due to the fact that we only have three icons, I would rather just import them without lazy loading.
import copy from './icons/Copy.svg?raw';
...
2 participants
Description:
ButtoncomponentNumberInputcomponentMessageBubblecomponentNameGeneratorcomponentTextAreacomponentIconcomponent