Combine index menu defs #940
Conversation
Use useMenuEntriesState to create the index menu options for events/id/index and organizations/id/index. Add idStr to prevent NaN from breaking urls.
Thank you for the pull request!The activist team will do our best to address your contribution as soon as we can. The following is a checklist for maintainers to make sure this process goes as well as possible. Feel free to address the points below yourself in further commits if you realize that actions are needed :) If you're not already a member of our public Matrix community, please consider joining! We'd suggest using Element as your Matrix client, and definitely join the General and Development rooms once you're in. Also consider joining our bi-weekly Saturday dev syncs. It'd be great to have you! Maintainer checklist
|
✅ Deploy Preview for activist-org ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
Thanks so much for the PR, @t3azr! Would you want to give the frontend warnings a quick look? I can also support it needed! 😊 |
|
sorry for taking so long !! im pretty sure the only errors were because I forgot to use prettier to format the file , but if there's anything else wrong please contact me !! hopefully these changes have been updated and work !! |
| @@ -3,8 +3,11 @@ import { IconMap } from "~/types/icon-map"; | |||
| const createMenuEntry = (label: string, basePath: string, iconURL: string) => { | |||
| const { locale } = useI18n(); | |||
| const router = useRouter(); | |||
| // here, afaik, this is returning NaN as the id of a given organization is alphanumeric -- not a number. | |||
| // added the raw id which can be used with the routeURL , while maintaining the number version | |||
| const idStr = router.currentRoute.value.params.id; | |||
| const id = Number(router.currentRoute.value.params.id); | |||
There was a problem hiding this comment.
Ya this is a holdover from prior setups where the route URL was an integer. Nice catch! Let's just set id to your value - without the Number() :)
There was a problem hiding this comment.
All's working really well, @t3azr! Really appreciate you taking the time to make these changes and that you reacted to the tests as well! Sorry my review was a bit on the late side :) Getting back into things after needing to focus elsewhere 😊 Hope you pick up another issue soon!
[x] This pull request is on a separate branch and not the main branch
Use useMenuEntriesState to create the index menu options for events/id/index and organizations/id/index. Add idStr to prevent NaN from breaking urls.
Used useMenuEntriesState to create the menu options for both events and organizations. Changed the id used to create URL routes in useMenuEntriesState .
When tested using an example organization, redirected to the correct URL asfaik.
Hope this is good!! I think the commit name may have become messed up when trying to commit if you need that fixed, thank you !!