feat: fixed admin app navigation issue #1298
Conversation
There was a problem hiding this comment.
Good work @BhuwanChandra and sorry for the delay in the review.
I've left a few comments nothing major. Please check them out 🙂
It's about the plugin implementation approach.
The point of having plugin type is that you can have many plugins with common functionality.
That’s why type can be reused, and name makes that particular plugin unique.
| import { i18n } from "@webiny/app/i18n"; | ||
| const t = i18n.ns("app-admin/navigation"); | ||
|
|
||
| const plugin: AdminMenuCommunityPlugin = { |
There was a problem hiding this comment.
@BhuwanChandra I think instead of creating a plugin type for each list item. We should create some plugins with type say "admin-drawer-footer-menu". For example,
{
type: "admin-drawer-footer-menu",
name: "admin-drawer-footer-menu-source",
render() {
// Some custom component
return <MenuItemSource />;
}
}And then render all "admin-drawer-footer-menu" plugins inside packages/app-admin/src/plugins/Menu/Navigation/index.tsx:67
| import { i18n } from "@webiny/app/i18n"; | ||
| const t = i18n.ns("app-admin/navigation"); | ||
|
|
||
| const plugin: AdminMenuSourcePlugin = { |
There was a problem hiding this comment.
Please do the same as suggested above :)
| @@ -41,6 +39,22 @@ const Navigation = () => { | |||
| return null; | |||
| }, []); | |||
|
|
|||
| const community = useMemo(() => { | |||
There was a problem hiding this comment.
@BhuwanChandra after doing the suggested change. (please check the other comment)
You can simply use the following code snippet:
const footerMenuPlugins = getPlugins<AdminDrawerFooterMenu>("admin-drawer-footer-menu");And then Iterate over these plugins and render the component. Like we are doing for menu plugins here
| return ( | ||
| <> | ||
| <a | ||
| href="https://community.webiny.com/" |
There was a problem hiding this comment.
@BhuwanChandra also please change Community with Slack as mentioned in the original issue.
This means changing the link, text, and icon of the menu item.
There was a problem hiding this comment.
Great work @BhuwanChandra 👍
Almost there 🚀
I've just left a comment, minor stuff. Please take a look.
PS: Code format check is also failing can you please also fix that. You can use this command: yarn run prettier:check 🙂
| const footerMenuPlugins = getPlugins<AdminDrawerFooterMenuPlugin>("admin-drawer-footer-menu"); | ||
|
|
||
| footerMenuPlugins && | ||
| sortBy(footerMenuPlugins, [p => p.order || 50, p => p.name]).forEach(plugin => { |
There was a problem hiding this comment.
@BhuwanChandra I don't think we need sortBy here as we don't define order property on the plugin itself.
There was a problem hiding this comment.
LGTM 🚀
Nicely done @BhuwanChandra 👍
Not sure why the checks are falling. We'll check and merge it soon.
Related Issue
Closes #1259
Your solution
changed the height and width of Api information icon
moved community section and source section to seperate plugins
How Has This Been Tested?
Manual Testing
Screenshots (if relevant):