electinth/dream-constitution

[component] Navigation bar

Closed this issue ยท 9 comments

image
have 2 themes, you can name it

Use in

Every page

Props

  • theme: name of style (up to you)

Behavior

When the hamburger menu is clicked, open list of topics fullscreen

image

Note

Topics data can be import from data/topics.ts

mixth commented

Done! I wonder if there should be a transition when the menu toggles. ๐Ÿค”

Note

Max the width of menu items at 544px.

Screen Shot 2021-07-25 at 5 27 46 PM

Need some minor updates according to the latest design. We need some mechanics to open the menu from outside the component which will be used in the result page like this button.

image

How to do it is the point.

  1. Emit the event but React is suck with this pattern as far as I researched LOL.
  2. Use variable through prop, then this component set the open menu function to this prop. Not sure if it's possible though.
  3. Create a method that can be accessed by the parent component somehow.

Not sure if you have any idea about this.

The rest is looking great! The transition would be nice but let ship the MVP first LOL

mixth commented

Hmmmm...

  • Listening to sibling or parent component's event looks weird to me.
  • Call children's function is also strange. But, I have seen this before in Angular or pure custom-element. Not sure if it's anti-pattern or not.

Controlling by prop seems to make the most sense.
The NavigationBar have a property to show menu, showMenu, and accept Handler, ToggleButtonClicked, that receives a custom event to toggle the menu. The parent of NavigationBar can now control the state of its menu.

I'm thinking of the way to make things easier. We could have a component called PageLayout to handle max-width and also NavigationBar and Footer. In this case, the logic to create and set state of showMenu shall be in this component as well.

With properties like this:

interface PageLayoutProps {
  navTheme: 'black' | 'transparent';
  className?: string;
  navClassName?: string;
  showFooter?: boolean; // Default at true
  showMenu?: boolean; // This and below are for manually controlling the state of menu
  toggleMenuButtonClicked?: EventHandler<CustomEvent> 
}

Normal usage:

<PageLayout theme="black">
  <div>Hello</div>
</PageLayout>

Manual control usage:

<PageLayout theme="black" showMenu={menu} toggleMenuButtonClicked={() => { setMenu(...) }}>
  <Button onClick={() => setMenu(true) }>Toggle Menu</Button>
</PageLayout>

Looks quite complicated, but I have no other ideas.
Feel free to reject the above thought since it comes from a person with too little experience with React. ๐Ÿ˜‚

Do let me know which direction to update this component. ๐Ÿ‘Œ


PS. There is some runtime error on the menu. I will track and resolve them tomorrow.

My bad for the event pattern. I was so confused and that solution is not making any sense ๐Ÿคฃ

I saw calling child function pattern in Svelte and Vue. React is possible according to https://stackoverflow.com/questions/37949981/call-child-method-from-parent but I don't have a strong opinion about this either.

React philosophy of this problem should be Lift the state up and let the parent of navbar hold is-opened state` as you suggested. One thing I'm curiuos about is, only the result page needs this requirement so the other pages don't care if the navbar is open. Is it possible to design a component that allows manual trigger the state but also be able to encapsulate the state itself if a parent doesn't need to do anything about it?

Lastly, the Page layout is not that necessary since it can't handle max-width (and padding) in our case. We have several sections with the different background colors, and we want max-width for just the content, not the background. (correct me if I'm wrong).

mixth commented

I think we can have showMenu and toggleMenuButtonClicked as props for NavigationBar. Without them provided, the component then creates the state itself.

Is it plausible?


BTW, I have these classes declared globally while I created topic pages with max-width in this commit c7e7c96

Sound great to me! and your .section > * is a clever one.

mixth commented

Will let you know when this is ready ๐Ÿ™Œ

mixth commented

done with e71ca30. Example added in index page.

cc @Th1nkK1D