activist
activist copied to clipboard
activist-org
Combine index menu option definitions
Terms
- [X] I have searched open and closed feature requests
- [X] I agree to follow activist's Code of Conduct
Description
The definitions for index menu options were copied in some cases, and it would be great if they could be combined. Specifically on the following two pages:
we have organizationButtons and eventButtons respectively that should likely be using the logic in useMenuEntriesState.
Contribution
Would be great if someone wanted to help with this! Happy to assist as needed or get to it myself eventually 😊
Thanks for checking the issues and writing in so many, @tasawar-hussain! Assigning you here, and please let us know if there's something we can do to help!
Hey @tasawar-hussain 👋 Checking in with you here :) Is there something we can do to support?
Nothing right now, will start this week, and get back to you if there is something
Hello!! I'd like to try at this if that's okay : ). If I'm understanding/reading it right -- in both the pages, the organisation menu buttons are being manually created, and instead you want it to be done through useMenuEntriesState? Please feel free to correct me // add anything on !!
Yes, exactly that, @t3azr! Thanks so much for your willingness to work on this! :)
Give the contribution guide a Quick Look for the sections you'll need for this, and please let us know if there's anything you can do to help!
could i just ask , in the pages provided , i can only see organisedButtons , no eventButtons . did the original post mean to also change it to eventButtons in the event page or am i missing something ? :)
Hey @t3azr 👋 There's a chance that the original eventButtons got deleted, but we can take a look at this later. Do you want to implement for organizationButtons and we can check further in the PR once that's ready? :)
Hello !! I've gotten most of it done now, I think. Just final checks. However: I came across something and I can't tell if I've misunderstood the code or if it's a bug.
When menu entries are created it Numbers() the route value to create the ID. However, for projects and events, the route is an alphanumeric string - so this was outputting NaN. This was causing issues as the routeURL and such use that ID to create the full link/route.
To kind of bandaid this, I've just added an intermediary idStr, which is the same as the ID, but without the Number(), as I didn't want to remove it entirely in case it's important/used elsewhere. So the routeURL uses idStr instead of the normal id.
However: I'm not sure where else this function is used, so I wanted to make you aware of it in case it breaks any other sites where Number() is needed!!
The ID for each entry is left as NaN, and afaik, this hasn't caused any issues -- the buttons still redirect to its respective page. Thanks!! Also, just a P.S : ignore my question about eventButtons, I had only quickly skimmed the code and completely missed it -- it is there !!
Hey @t3azr 👋 Really sorry for the delay here. I've been traveling/seeing friends and family and just barely getting to critical issues and reviews. I do have this in mind, and am on my way back home today!
Hope all's well :)
thanks for your response ! no worries - i’ve just been on holiday as well :) hope all is well for you too !
To kind of bandaid this, I've just added an intermediary idStr, which is the same as the ID, but without the Number(), as I didn't want to remove it entirely in case it's important/used elsewhere. So the routeURL uses idStr instead of the normal id.
However: I'm not sure where else this function is used, so I wanted to make you aware of it in case it breaks any other sites where Number() is needed!!
Am doing well, thanks :) In regards to this, I'm very happy to check out a PR with these changes and can do any cleanup that's needed 😊 Thanks so much for detailing what you're changing!
Closed by #940 :) Thanks so much for the work here, @t3azr! Really appreciate it :) Let us know if another issue sounds appealing!