-
Notifications
You must be signed in to change notification settings - Fork 1
Description
When adding the type hints to #69 I realized that I don't want the get_items method to return a list of RenderedNavItem and have someone have to deal with the overhead of an additional extra class. Or add additional typing to return a list of RenderedNavItem or NavGroup | NavItem. That's just sloppy and unnecessary.
Instead, the NavGroup and NavItem should have a render method with any associated helper methods to go along with that. It would bring them more in line with the Nav as well and its render method.
I never really liked the solution I came up with regarding the rendering of the items, but the extra RenderedNavItem class seemed the most simple solution at the time. After sitting with it for a while, this way seems cleaner and somehow more simple. Not sure how I didn't see it the first go around.
- Add
rendermethods to bothNavGroupandNavItem- Instead of an extra
RenderedNavItemclass, it could instead be a mixin that both of them inherit to cut down on duplication. Though the number of helper methods responsible for rendering is not that many, so that maybe overkill and just some small duplication would be better. 🤷♂️ Table that for now until PR review.
- Instead of an extra
- Remove
RenderedNavItem - Change
Nav.get_itemsmethod to call therendermethod for each item. Additionally, adjust the type hint tolist[NavGroup | NavItem]