Skip to content

Conversation

@ctindel
Copy link

@ctindel ctindel commented Dec 21, 2019

This is a port of the 2-year old PR #111 so it can be merged cleanly originally written by @ringods

This PR offers:

  • dropdown menu with single list of menu items
  • dropdown menu with 4 columns of menu items, grouped in sections
  • dropdown menu with 2 column wide image, followed by 2 columns of menu items grouped in sections.

Part of #101

@ctindel
Copy link
Author

ctindel commented Dec 30, 2019

@GeorgeWL @ryanfox1985 Hi, just wanted to see if we could get this merged at some point, I know it was mentioned recently on the 2 year old PR that you'd accept it if the merge conflicts were removed.

@ctindel
Copy link
Author

ctindel commented Feb 21, 2020

@ryanfox1985 @GeorgeWL any chance of getting this merged? Wasn't sure if it was blocked behind something else. Thanks!

Copy link
Contributor

@GeorgeWL GeorgeWL left a comment

Choose a reason for hiding this comment

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

looks fine @devcows

@ryanfox1985
Copy link
Contributor

ryanfox1985 commented Feb 25, 2020

I will test it during the week.

to indicate in which column a section will be put in. Within a column, the `weight` value is respected to show the
sections top to bottom.

Use to the unique section identifier (e.g. `section.ap-blog`) as the `parent` value to add a menu item to a specific
Copy link
Contributor

Choose a reason for hiding this comment

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

Use to the unique ...

I see a typo now in my own 2-year old work. 😜

Copy link
Contributor

Choose a reason for hiding this comment

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

ha, removing just one word isn't the worst thing 😛

@salim-b
Copy link
Collaborator

salim-b commented Feb 25, 2020

This PR has some serious deficiencies which should be solved before merging:

@ctindel
Copy link
Author

ctindel commented Feb 25, 2020

@salim-b I may be out of my element here since I was just forward-porting someone else's code and it worked ok on my site. I'm not sure what you mean by navigation highlighting. I'm running this code on my site as i roll over the top level menu items as well as the sub menu items they do highlight. Is there another type of highlighting is broken?

That said I didn't realize there was a version of universal theme that had this already unreleased so if you don't think it's worth taking the change maybe we can just drop it.

@salim-b
Copy link
Collaborator

salim-b commented Feb 25, 2020

I'm running this code on my site as i roll over the top level menu items as well as the sub menu items they do highlight. Is there another type of highlighting is broken?

Yes, the respective top level item should stay highlighted when the corresponding page is visited 😉

I didn't realize there was a version of universal theme that had this already unreleased

I don't really understand what you mean by "already unreleased", but there is ongoing effort to port this whole hugo theme to the new Universal v2 upstream theme in #246

so if you don't think it's worth taking the change maybe we can just drop it.

Personally I don't need the extended menus, so I probably won't put much work into improving this PR. But since there's no ETA for #246, it might make sense for others to do that work...

@ryanfox1985
Copy link
Contributor

shall I close #101 in favour of this PR?

@ryanfox1985
Copy link
Contributor

ryanfox1985 commented Aug 4, 2021

I'm running this code on my site as i roll over the top level menu items as well as the sub menu items they do highlight. Is there another type of highlighting is broken?

it's not working well the highlighting menu.

@ryanfox1985
Copy link
Contributor

I updated #111 into #307

@ryanfox1985
Copy link
Contributor

closed in favour of #307

@ryanfox1985 ryanfox1985 closed this Aug 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants