toss / overlay-kit

The simplest and most intuitive way to manage overlays in React.
https://overlay-kit.slash.page
MIT License
296 stars 31 forks source link

`overlay.close` and `overlay.unmount`'s argument, parameter name matching #38

Closed jgjgill closed 4 months ago

jgjgill commented 4 months ago

When I read docs, overlay.close and overlay.unmount arguments are overlayId.

스크린샷 2024-07-05 오후 6 55 52

But in the code, they are id as parameters.

events.ts

스크린샷 2024-07-05 오후 7 01 24

I think it would be more consistent if it was overlayId. what do you think?

jungpaeng commented 4 months ago

I don’t think the issue you pointed out significantly affects the usability of the library. However, I believe that small details like these contribute to the overall quality. Therefore, I agree that it would be better to align with the documentation for consistency.

I will make the changes. Thank you for pointing this out.

jgjgill commented 4 months ago

I don’t think the issue you pointed out significantly affects the usability of the library.

I agree. I thought the feature code was changing, as opposed to the test code or documentation, so I created an issue instead of writing a PR right away, even though it's a minor detail. 😂 If you haven't worked on it yet, can I write a PR and contribute to it? 🧐

jungpaeng commented 4 months ago

If you haven't worked on it yet, can I write a PR and contribute to it? 🧐

Sure! Contributing through a PR would be very helpful. Thank you!

jgjgill commented 4 months ago

Ok! I'm learning a lot thanks to you! 👍