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

Kuwait Comp: Bottom Navbar #2508

Merged
merged 24 commits into from
Dec 30, 2024
Merged

Kuwait Comp: Bottom Navbar #2508

merged 24 commits into from
Dec 30, 2024

Conversation

FaithDaka
Copy link
Collaborator

@FaithDaka FaithDaka commented Nov 6, 2024

PR Checklist

  • PR title descriptive (can be used in release notes)

Description

Includes a dynamic navigation bar component for the Kuwait app.

Git Issues

Closes #

Screenshots/Videos

comp_plh_bottom_nav

Screenshot 2024-12-24 at 12 52 15

Parameter list (making use of the changes in #2631):
Screenshot 2024-12-24 at 12 51 53

Screenshot 2024-12-24 at 12 52 36

@FaithDaka
Copy link
Collaborator Author

@chrismclarke @jfmcquade

I have a few things I want to achieve in this PR:

  1. Creating a new layout component for the Kuwait theme. Given that the design doesn't have a static header, footer, margins or padding, there's need to destructure the layout defined in app.component.html. I created a plh_kids_kw folder within the packages/components in order for it to have a dedicated module.
    I don't know if I'm better off creating a new skin or if we can explore this option?
  2. Routing to the 3 pages represented by the navbar items. For the debug app I wanted a new footer where the home button mapped to the theme page, library button mapped to the templates page and settings button mapped to the side-menu items. Is that reasonable?
  3. The actual navbar design.

Item 3 has been done (with some static styling). It's 1 and 2 that are a WIP and need insights

@FaithDaka FaithDaka self-assigned this Nov 6, 2024
@FaithDaka FaithDaka added the feature Work on app features/modules label Nov 7, 2024
@FaithDaka FaithDaka added this to the ParentApp Kids KW Dec 2024 milestone Nov 7, 2024
@chrismclarke
Copy link
Member

@chrismclarke @jfmcquade

I have a few things I want to achieve in this PR:

  1. Creating a new layout component for the Kuwait theme. Given that the design doesn't have a static header, footer, margins or padding, there's need to destructure the layout defined in app.component.html. I created a plh_kids_kw folder within the packages/components in order for it to have a dedicated module.
    I don't know if I'm better off creating a new skin or if we can explore this option?
  2. Routing to the 3 pages represented by the navbar items. For the debug app I wanted a new footer where the home button mapped to the theme page, library button mapped to the templates page and settings button mapped to the side-menu items. Is that reasonable?
  3. The actual navbar design.

Item 3 has been done (with some static styling). It's 1 and 2 that are a WIP and need insights

Thanks Faith, all sounds like good options to have at the core app layout level.

Am I right in thinking that only the main section would have routing included and the other two sections single templates? If so it may be worth considering using something like a slider/modals/overlays to display the section content so that a user will always be returned to the same place when resuming main section (it never actually is removed from the dom). This wouldn't really work if the other sections were routable, in which case we could still look into using angular child routers.

Remind me @jfmcquade how we've handled bottom navbars in the past and whether we've had a similar use case?

@jfmcquade
Copy link
Collaborator

Apologies, I've not had time to properly engage with this fully, but I think your plan sounds good, @FaithDaka

Remind me @jfmcquade how we've handled bottom navbars in the past and whether we've had a similar use case?

I believe we have one way to configure a footer: via the APP_FOOTER_DEFAULTS.templateName parameter, which can be set either in a deployment config or in a skin. Then the convention has been to use a specially designed component, navigation_bar within the template whose name gets passed to that property. The navigation_bar component itself does not contain any specific features that make it act as a footer to the app, that's all handled by the core app code, which displays any template that is passed within an ion-footer element defined within the app component template.

Since the footer is applied at the deployment config/skin level, we don't currently have any functionality for displaying a footer on some pages and not others.

@jfmcquade
Copy link
Collaborator

jfmcquade commented Dec 24, 2024

Now that #2631 is merged, we can properly test this component in situ. I've updated the component demo template and will update the PR description.

@FaithDaka there was an issue with the active link logic. We can't track isActive at the component level, since the component contains buttons for multiple links, only one of which should be active at a time. I've fixed this using the RouterLinkActive directory to track the isActive state of each button, see my changes in these commits.

@esmeetewinkel I believe this PR should now be ready for your functional testing

Copy link
Collaborator

@esmeetewinkel esmeetewinkel left a comment

Choose a reason for hiding this comment

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

Functional test passed.

nit (non-blocking): I was surprised to find that the navigation only works correctly when the plh_bottom_nav component is used in the footer - clicking any of the icons directly in the template leads to the app's home screen.

Screenity.video.-.Dec.30.2024.1.mp4

@jfmcquade jfmcquade merged commit dacdd46 into master Dec 30, 2024
8 checks passed
@jfmcquade jfmcquade deleted the feat/bottom-nav branch December 30, 2024 17:13
@esmeetewinkel esmeetewinkel mentioned this pull request Jan 27, 2025
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Work on app features/modules test - preview Create a preview deployment of the PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants