Skip to content

Conversation

Cr0zy07
Copy link
Collaborator

@Cr0zy07 Cr0zy07 commented Dec 11, 2023

Description

Linked Issues

cc #358

Additional context

Copy link
Contributor

github-actions bot commented Dec 11, 2023

Thank you for following the naming conventions! 🙏

Comment on lines +37 to +42
validator: (value: string): boolean => {
if (!isValidOrientation(value))
console.error(getInvalidOrientationError(String(value), SEPARATOR_NAME))

return true
},
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I moved the prop validation from the component to the props itself. However, since the validator does not support a custom error message, I used console.error instead of using throw new Error.

Reference

Copy link
Contributor

@productdevbook productdevbook left a comment

Choose a reason for hiding this comment

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

I added useListener and i send commit.

Another great job and a great works. Thank you for your work.

@Cr0zy07
Copy link
Collaborator Author

Cr0zy07 commented Dec 11, 2023

I added useListener and i send commit.

Another great job and a great works. Thank you for your work.

Actually, there is no emits in that component. That's why I didn't use useListener😅>

@productdevbook
Copy link
Contributor

The reason for this is that because you use emits at the top, the transition to the bottom is not applied, the emits remain in the script if the user sends emits.

@Cr0zy07
Copy link
Collaborator Author

Cr0zy07 commented Dec 11, 2023

The reason for this is that because you use emits at the top, the transition to the bottom is not applied, the emits remain in the script if the user sends emits.

Yes, I understand that. But that component doesn't accept any emits it's actually empty emits: { }🙄

@productdevbook
Copy link
Contributor

I'm torn on this one, you're right, they both work.

@productdevbook
Copy link
Contributor

productdevbook commented Dec 12, 2023

shall we merge ?

@Cr0zy07
Copy link
Collaborator Author

Cr0zy07 commented Dec 12, 2023

shall we merge ?

Yes, please!

@productdevbook productdevbook merged commit eacee97 into oku-ui:main Dec 13, 2023
@github-actions github-actions bot mentioned this pull request Dec 8, 2023
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.

2 participants