implementing navbar custom buttons #2362

cphelefu · 2024-07-11T07:30:19Z

Description

This PR refactors Navbar component. It does the following;

Adds ability to add custom button (with an onClick action).
Adds custom buttons that open up a panel.

Fixes #2180

Type of change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce.

https://cphelefu.github.io/geoview/add-panels.html

Checklist:

I have build (rush build) and deploy (rush host) my PR
I have connected the issues(s) to this PR
My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
My changes generate no new warnings
I have created new issue(s) related to the outcome of this PR is needed
~~I have made corresponding changes to the documentation~~
~~I have added tests that prove my fix is effective or that my feature works~~
~~New and existing unit tests pass locally with my changes~~

This change is

…elefu/geoview into 2180-implement-navbar-custom-buttons

jolevesq

Reviewed 3 of 5 files at r1, 3 of 3 files at r2, all commit messages.
Reviewable status: all files reviewed, 13 unresolved discussions (waiting on @cphelefu and @kaminderpal)

packages/geoview-core/public/templates/add-panels.html line 144 at r2 (raw file):

            const groupName = navbarButtonGroupNameInput.value;

            if(groupName === '') {

Space between if and (

packages/geoview-core/src/core/components/nav-bar/nav-bar.tsx line 24 at r2 (raw file):

};

type DefaultNavbar = 'fullScreen' | 'location' | 'home' | 'zoomIn' | 'zoomOut';

Type should be map-schema-type in api folder. If constant is needed, it should be in constant.ts

packages/geoview-core/src/core/components/nav-bar/nav-bar.tsx line 60 at r2 (raw file):

  const [buttonPanelGroups, setButtonPanelGroups] = useState<NavButtonGroups>(defaultButtonGroups);

  useEffect(() => {

Add logger to useEffect

packages/geoview-core/src/core/components/nav-bar/nav-bar.tsx line 65 at r2 (raw file):

      displayButtons = { ...displayButtons, fullScreen: 'fullScreen' };
    }
    if (navBarComponents.includes('location')) {

Blank line between if

packages/geoview-core/src/core/components/nav-bar/nav-bar.tsx line 68 at r2 (raw file):

      displayButtons = { ...displayButtons, location: 'location' };
    }
    if (navBarComponents.includes('home')) {

Blank line between if

packages/geoview-core/src/core/components/nav-bar/nav-bar.tsx line 83 at r2 (raw file):

      logger.logTraceUseCallback('NAV-BAR - addButtonPanel');

      const d = {

What is d for? Maybe a more meaningful name?

packages/geoview-core/src/core/components/nav-bar/nav-bar.tsx line 101 at r2 (raw file):

    (sender: NavBarApi, event: NavBarRemovedEvent) => {
      logger.logTraceUseCallback('NAV-BAR - handleRemoveButtonPanel');
      setButtonPanelGroups((prevState) => {

Blank line between logger and function

packages/geoview-core/src/core/components/nav-bar/nav-bar-panel-button.tsx line 14 at r2 (raw file):

/**
 * Navbar modal component

Not modal, it is popover

packages/geoview-core/src/core/components/nav-bar/nav-bar-panel-button.tsx line 16 at r2 (raw file):

 * Navbar modal component
 *
 * @returns {JSX.Element} the export modal component

not export modal

packages/geoview-core/src/core/components/nav-bar/nav-bar-panel-button.tsx line 19 at r2 (raw file):

 */
export default function NavbarPanelButton({ buttonPanel }: NavbarPanelButtonType): JSX.Element {
  const theme = useTheme();

logger.logTraceRender( at the start of component

packages/geoview-core/src/core/components/nav-bar/nav-bar-panel-button.tsx line 79 at r2 (raw file):

          <DialogTitle>{(buttonPanel.panel?.title as string) ?? ''}</DialogTitle>
          <DialogContent dividers>
            <Typography dangerouslySetInnerHTML={{ __html: buttonPanel?.panel?.content ?? '' }} />

For now but content can be a react component pass by props, not only sting

packages/geoview-core/src/core/components/nav-bar/nav-bar-style.ts line 28 at r2 (raw file):

    position: 'relative',
    pointerEvents: 'auto',
    // justifyContent: 'end',

remove dead code

packages/geoview-core/src/core/components/nav-bar/nav-bar-style.ts line 32 at r2 (raw file):

    padding: 5,
    flexDirection: 'column',
    // flexWrap: 'wrap',

remove dead code

kaminderpal

Reviewable status: all files reviewed, 13 unresolved discussions (waiting on @cphelefu and @jolevesq)

packages/geoview-core/src/core/components/nav-bar/nav-bar-panel-button.tsx line 79 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

For now but content can be a react component pass by props, not only sting

Also we can use HtmlToReact npm package instead of dangerouslySetInnerHTML

cphelefu · 2024-07-12T11:44:48Z

packages/geoview-core/src/core/components/nav-bar/nav-bar.tsx line 24 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

Type should be map-schema-type in api folder. If constant is needed, it should be in constant.ts

This type is just specific to this file; it can even just be converted to a string. Just added it to reduce errors of wrong key.

cphelefu

Reviewable status: all files reviewed, 13 unresolved discussions (waiting on @jolevesq)

packages/geoview-core/public/templates/add-panels.html line 144 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

Space between if and (

Done.

packages/geoview-core/src/core/components/nav-bar/nav-bar.tsx line 60 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

Add logger to useEffect

Done.

packages/geoview-core/src/core/components/nav-bar/nav-bar.tsx line 68 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

Blank line between if

Done.

packages/geoview-core/src/core/components/nav-bar/nav-bar.tsx line 83 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

What is d for? Maybe a more meaningful name?

Done.

packages/geoview-core/src/core/components/nav-bar/nav-bar.tsx line 101 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

Blank line between logger and function

Done.

packages/geoview-core/src/core/components/nav-bar/nav-bar-panel-button.tsx line 14 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

Not modal, it is popover

Done.

packages/geoview-core/src/core/components/nav-bar/nav-bar-panel-button.tsx line 16 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

not export modal

Done.

packages/geoview-core/src/core/components/nav-bar/nav-bar-panel-button.tsx line 19 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

logger.logTraceRender( at the start of component

Done.

packages/geoview-core/src/core/components/nav-bar/nav-bar-style.ts line 28 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

remove dead code

Done.

packages/geoview-core/src/core/components/nav-bar/nav-bar-style.ts line 32 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

remove dead code

Done.

cphelefu

Reviewable status: 2 of 6 files reviewed, 13 unresolved discussions (waiting on @jolevesq)

packages/geoview-core/src/core/components/nav-bar/nav-bar-panel-button.tsx line 79 at r2 (raw file):

Previously, kaminderpal (Kamy) wrote…

Also we can use HtmlToReact npm package instead of dangerouslySetInnerHTML

Done.

cphelefu

Reviewable status: 2 of 8 files reviewed, 13 unresolved discussions (waiting on @cphelefu and @jolevesq)

packages/geoview-core/src/core/components/nav-bar/nav-bar.tsx line 65 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

Blank line between if

Done.

jolevesq

Reviewed 1 of 4 files at r3, 5 of 5 files at r4, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @cphelefu)

packages/geoview-core/src/core/components/nav-bar/nav-bar.tsx line 24 at r2 (raw file):

Previously, cphelefu (Christopher Phelefu) wrote…

This type is just specific to this file; it can even just be converted to a string. Just added it to reduce errors of wrong key.

But component should reuse the type made for them by the map-schema-type. Because at the end, map-schema type is the one who validates if the component is good or not. The map-schema-Type must be aligned with the component. The zoom always come with zoomIn/zoomOut in one group

/** Valid values for the navBar array. */
export type TypeValidNavBarProps = 'zoom' | 'fullscreen' | 'home' | 'location';

packages/geoview-core/src/core/components/nav-bar/nav-bar-panel-button.tsx line 1 at r4 (raw file):

/* eslint-disable @typescript-eslint/no-explicit-any */

No global eslint, we need to use them were needed

cphelefu

Reviewable status: 4 of 8 files reviewed, 2 unresolved discussions (waiting on @jolevesq)

packages/geoview-core/src/core/components/nav-bar/nav-bar.tsx line 24 at r2 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

But component should reuse the type made for them by the map-schema-type. Because at the end, map-schema type is the one who validates if the component is good or not. The map-schema-Type must be aligned with the component. The zoom always come with zoomIn/zoomOut in one group
/** Valid values for the navBar array. */
export type TypeValidNavBarProps = 'zoom' | 'fullscreen' | 'home' | 'location';

Done. Removed that code.

packages/geoview-core/src/core/components/nav-bar/nav-bar-panel-button.tsx line 1 at r4 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

No global eslint, we need to use them were needed

Done.

jolevesq

Reviewed all commit messages.
Reviewable status: 4 of 8 files reviewed, 2 unresolved discussions (waiting on @cphelefu)

packages/geoview-core/src/core/components/nav-bar/nav-bar-panel-button.tsx line 84 at r5 (raw file):

          <DialogTitle sx={sxClasses.popoverTitle}>{(buttonPanel.panel?.title as string) ?? ''}</DialogTitle>
          <DialogContent>
            <HtmlToReact htmlContent={(buttonPanel?.panel?.content ?? '') as string} />

It may not be a tring,. keep the any but add the eslint line escape here

jolevesq

Reviewed 2 of 4 files at r5, 1 of 1 files at r6, 1 of 1 files at r7, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @cphelefu)

jolevesq

Reviewed 2 of 2 files at r8, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @cphelefu)

cphelefu added 4 commits July 11, 2024 03:27

implementing navbar custom buttons

6355bd5

Merge branch 'develop' into 2180-implement-navbar-custom-buttons

0703010

post build & fixing errors

8c0d74d

fix buttons

2fa2374

cphelefu marked this pull request as ready for review July 11, 2024 12:59

cphelefu self-assigned this Jul 11, 2024

jolevesq requested review from jolevesq and kaminderpal July 11, 2024 13:25

jolevesq mentioned this pull request Jul 11, 2024

2180 navbar panel #2348

Closed

11 tasks

cphelefu added 5 commits July 11, 2024 12:53

Merge branch 'develop' into 2180-implement-navbar-custom-buttons

afd5b71

fix based on feedback

51b2e0d

Merge branch '2180-implement-navbar-custom-buttons' of github.com:cph…

f442e2b

…elefu/geoview into 2180-implement-navbar-custom-buttons

minor changes

dbecbf2

minor fixes

4cb364a

jolevesq requested changes Jul 11, 2024

View reviewed changes

kaminderpal reviewed Jul 11, 2024

View reviewed changes

cphelefu commented Jul 12, 2024

View reviewed changes

code review feedback

9347109

cphelefu commented Jul 12, 2024

View reviewed changes

cphelefu added 2 commits July 12, 2024 08:03

code review feedback

4bd9ae2

fix popper header

19f8dbf

cphelefu commented Jul 12, 2024

View reviewed changes

jolevesq requested changes Jul 12, 2024

View reviewed changes

cphelefu added 2 commits July 12, 2024 09:37

fixing title border color

83c0503

fix the according to comments

7dcd850

cphelefu commented Jul 12, 2024

View reviewed changes

jolevesq requested changes Jul 12, 2024

View reviewed changes

cphelefu added 2 commits July 12, 2024 10:55

adding back eslint and converting to any

2d1d68a

reverting back to old code

275fdb8

jolevesq approved these changes Jul 12, 2024

View reviewed changes

cphelefu added 2 commits July 12, 2024 12:35

Merge branch 'develop' into 2180-implement-navbar-custom-buttons

98b9dd1

switching from popover to popper

8e88e58

jolevesq approved these changes Jul 12, 2024

View reviewed changes

jolevesq merged commit 5a2880b into Canadian-Geospatial-Platform:develop Jul 12, 2024
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

implementing navbar custom buttons #2362

implementing navbar custom buttons #2362

cphelefu commented Jul 11, 2024 •

edited

Loading

jolevesq left a comment

kaminderpal left a comment

cphelefu commented Jul 12, 2024

cphelefu left a comment

cphelefu left a comment

cphelefu left a comment

jolevesq left a comment

cphelefu left a comment

jolevesq left a comment

jolevesq left a comment

jolevesq left a comment

implementing navbar custom buttons #2362

implementing navbar custom buttons #2362

Conversation

cphelefu commented Jul 11, 2024 • edited Loading

Description

Type of change

How Has This Been Tested?

Checklist:

jolevesq left a comment

Choose a reason for hiding this comment

kaminderpal left a comment

Choose a reason for hiding this comment

cphelefu commented Jul 12, 2024

cphelefu left a comment

Choose a reason for hiding this comment

cphelefu left a comment

Choose a reason for hiding this comment

cphelefu left a comment

Choose a reason for hiding this comment

jolevesq left a comment

Choose a reason for hiding this comment

cphelefu left a comment

Choose a reason for hiding this comment

jolevesq left a comment

Choose a reason for hiding this comment

jolevesq left a comment

Choose a reason for hiding this comment

jolevesq left a comment

Choose a reason for hiding this comment

cphelefu commented Jul 11, 2024 •

edited

Loading