Skip to content
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

[PR] Adding dynamic menu #5

Merged
merged 56 commits into from
Jun 21, 2023
Merged

[PR] Adding dynamic menu #5

merged 56 commits into from
Jun 21, 2023

Conversation

LuchoTurtle
Copy link
Member

closes #4

This PR adds a menu that is dynamic, meaning it uses the information stemming from a json object to render the contents.
The json object consists of MenuItems (with an id, title and tiles - a list of MenuItems below).

@LuchoTurtle LuchoTurtle added documentation Improvements or additions to documentation enhancement New feature or enhancement of existing functionality in-progress An issue or pull request that is being worked on by the assigned person flutter Flutter related issues epic A feature idea that is large enough to require a sprint (5 days) or more and has smaller sub-issues. labels Apr 27, 2023
@LuchoTurtle LuchoTurtle self-assigned this Apr 27, 2023
@LuchoTurtle LuchoTurtle marked this pull request as draft April 27, 2023 22:11
@LuchoTurtle
Copy link
Member Author

I've managed to get the contents of json object to be rendered on the side menu drawer. Looking to Future-proof this and check how reorder could be implemented.

@nelsonic
Copy link
Member

In principal we will use the same position-based ordering for the drawer menu
so if you can get the drag-and-drop re-ordering working in Flutter-land that's awesome! 🤞

@LuchoTurtle
Copy link
Member Author

Reordering should now work locally. Meaning it is not saving the changes to the json file. Reordering only works on the items on the same level, though.

Screen.Recording.2023-04-28.at.20.04.30.mov

@nelsonic nelsonic mentioned this pull request May 2, 2023
10 tasks
@LuchoTurtle
Copy link
Member Author

I've managed to increase the code coverage to near 98%. Will tackle on getting it to 100%.

I've successfully tested dragging and dropping. Flutter's longPress doesn't work, so I had to create a custom function using startGesture to simulate drag an drop on widget tests.

image

@LuchoTurtle
Copy link
Member Author

Fixed a few bugs regarding reordering and fixed the decoration border that was overlapping on nested elements. Added both of these to the README.
Also fixed the CI to use the latest version of Flutter. By using collection, flutter_test was having conflicts with the collection package, so I had to force it to use the latest.

Not yet marking this for review because I want to add a simple navigation to a page displaying the menu item information. After that, everything should be thoroughly documented and ready for submission.

@LuchoTurtle LuchoTurtle marked this pull request as ready for review May 12, 2023 08:44
@LuchoTurtle LuchoTurtle assigned nelsonic and SimonLab and unassigned LuchoTurtle May 12, 2023
@LuchoTurtle LuchoTurtle added awaiting-review An issue or pull request that needs to be reviewed and removed in-progress An issue or pull request that is being worked on by the assigned person labels May 12, 2023
@LuchoTurtle
Copy link
Member Author

This should be done for review. Added navigation, tests to cover the full app and added a CI to fix typos on a separate PR. Because the changes added are substantial, there's a chance I got a typo here and there.

@nelsonic
Copy link
Member

@LuchoTurtle could you please record a short video/GIF of the final result. 🎥
@SimonLab if you have time please do an initial pass at reviewing this. 🙏

@LuchoTurtle
Copy link
Member Author

Screen.Recording.2023-05-12.at.09.57.19.mov

Here's an overview:

  • working i18n.
  • navigation on static menu items.
  • navigation on dynamic menu items.
  • showing support for icon, emoji and images that are formatted to 64x64px.
  • customization of dynamic menu text colour.
  • persistable reordering of dynamic menu items.

@nelsonic nelsonic requested a review from SimonLab May 16, 2023 09:01
@nelsonic nelsonic added the priority-1 Highest priority issue. This is costing us money every minute that passes. label May 24, 2023
@LuchoTurtle
Copy link
Member Author

Could this PR be merged? I believe it's complete and easily mergeable 🆗

Copy link
Member

@nelsonic nelsonic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I reviewed it at the time. 👀
Had asked @SimonLab to have a look, but I think he's focussed on other priority items. 👌
Merging. ✅

@nelsonic nelsonic merged commit de287d9 into main Jun 21, 2023
@nelsonic nelsonic deleted the fix-#4 branch June 21, 2023 09:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-review An issue or pull request that needs to be reviewed documentation Improvements or additions to documentation enhancement New feature or enhancement of existing functionality epic A feature idea that is large enough to require a sprint (5 days) or more and has smaller sub-issues. flutter Flutter related issues priority-1 Highest priority issue. This is costing us money every minute that passes.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[EPIC] Feat: Dynamic Navigation Menu
3 participants