-
Notifications
You must be signed in to change notification settings - Fork 213
Add vk-audiopad example #643
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
Conversation
✅ Deploy Preview for pr-fsd ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
illright
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi! Thanks for the example! The structure of the project is brilliant, I love it. One thing I'm concerned with is the set of entities.
active-track, track, and album are great entities in the business domain of your application, but card, snackbar, and content-tab — less so. They sound like generic UI components to me, and as such, I would expect them to be in shared/ui, or, if they're not generic, have a more specific name. Could you comment on that, please?
|
@illright hi!
About Looking at everything again, I removed the |
illright
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for clarifying and making edits :)
suggestion (non-blocking): how about taking it one step further? active-tab still doesn't feel like a business entity to me, it feels about the same as current-url or something else related to navigation. However, looking at its code and its usage, I do see the problem in just putting it into shared — it knows what pages there are. Still I think that Shared is semantically a more appropriate place for navigation than Entities. I'd approach this in one of two ways:
- Remove the knowledge about pages from the
current-tabatom. Have it accept an arbitrary string inopenPageand have it receive a list of valid pages on the App layer. This would hurt the type-safety of the code a bit - Just don't bother with this and put it into Shared exactly as it is. Yes, it knows about pages, well, so what. At least its API is ergonomic and type-safe. It's not like it's forbidden to put things in Shared that know even a tiny bit about the application, such a restriction would be way too impractical.
I would also consider putting notification into Shared as well, but that's not a big deal. Feel free to leave your code as it is, it's already a great example of FSD.
|
Thanks! Your comments were helpful and I feel like I now have a better understanding of FSD. With I moved the |
Background
I have developed an extension using FSD architecture, I think my example and solutions will be useful. The code is located at /packages/front.
Changelog
Added in /pages/examples/img/vkaudiopad.png
Added in /pages/examples/_config.ts