codyhouse / codyhouse-framework

A lightweight front-end framework for building accessible, bespoke interfaces.
https://codyhouse.co/
MIT License
1.16k stars 173 forks source link

Closing by default by clicking on a link in the dropdown components #51

Closed ruslankryl closed 4 years ago

ruslankryl commented 4 years ago

Hi, thank you for a great product! In many components with a dropdown list, the menu does not close when the link is clicked. For example,

  1. https://codyhouse.co/ds/components/app/mega-site-navigation click on Documentation
  2. https://codyhouse.co/ds/components/app/language-picker click on any option (closing in the demo, not in the copied code)
  3. https://codyhouse.co/ds/components/app/adaptive-navigation click on an option in the dropdown
  4. https://codyhouse.co/ds/components/app/menu click on the dropdown option

It doesn't seem very convenient, can you add a default closure? Thanks!

sebastiano-guerriero commented 4 years ago

Hi there! It's not closing because it's missing a real link's destination: in all our demos, the href value is #0. In a real project, you would replace it with a real link. When you click on a link, the page is refreshed, therefore you don't need to "close" the dropdown or the navigation, because you're moving to a new page.

ruslankryl commented 4 years ago

Hi there! It's not closing because it's missing a real link's destination: in all our demos, the href value is #0. In a real project, you would replace it with a real link. When you click on a link, the page is refreshed, therefore you don't need to "close" the dropdown or the navigation, because you're moving to a new page.

Hi, thank you for your answer. This is not quite true. Half of the components do not necessarily involve going to a new page. Also, this logic does not work in a SPA where the transition is made by means of a router (for example, https://github.com/ReactTraining/react-router or other libraries that perform routing based on the History API)

sebastiano-guerriero commented 4 years ago

I see your point. I'll look further into this. I don't think it would be a good idea to add extra javascript to all the components where an "active" state needs to be moved from one item to the other, or when a class needs to be triggered to "close" something (e.g., a dropdown or a navigation) to account for SPA. But we could include a snippet in the info page of these components. I'm not reopening the issue because it's not strictly related to the framework. Feel free to submit this support request to the dedicated changelog of the components that could use this improvements. We just need to figure out what would be the best way to integrate this feature👍

ruslankryl commented 4 years ago

But we could include a snippet in the info page of these components.

Sounds cool! Thanks