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

Link menus to the 'Configure' tab #54

Open
BWPanda opened this issue Jun 26, 2018 · 3 comments

Comments

@BWPanda
Copy link
Member

commented Jun 26, 2018

When you're at /admin/config/search/xmlsitemap/settings and click on a menu name under the 'Menu link' vertical tab, it takes you to menu's 'Edit links' tab (e.g. /admin/structure/menu/manage/main-menu/edit). Since you want to configure XML Sitemap, it should take you to the 'Configure' tab instead (e.g. /admin/structure/menu/manage/main-menu/configure).

@alexfinnarn alexfinnarn added this to the 1.0.4 milestone Jul 23, 2018

@alexfinnarn

This comment has been minimized.

Copy link
Contributor

commented Jul 23, 2018

So that link is gathered from entity_get_info(), however, the xmlsitemap_menu module already alters the "menu_link" entity info.

I've made a PR that change the links to go to "Configure", but I'm not sure that should be done as all other code depending on that link might break.

This issue also brings up a regression introduced by fixing #42. I placed the code for that into this PR, but the other modules need to be checked to make sure that they declare a hook to be included on the settings page.

@jenlampton

This comment has been minimized.

Copy link
Member

commented Jan 9, 2019

that link is gathered from entity_get_info() ... all other code depending on that link might break.

Can't we just update the link to configure for this instance, without changing something that affects anything else?

the other modules need to be checked to make sure that they declare a hook to be included on the settings page.

Do we know if there are any "other modules" yet, and if so, what they are?

@alexfinnarn

This comment has been minimized.

Copy link
Contributor

commented Jan 21, 2019

Do we know if there are any "other modules" yet, and if so, what they are?

That was for declaring hook_xmlsitemap_link_info() which was actually merged in for the xmlsitemap_menu module in another PR so I've removed that part. Looks like the modules that should be in that list are there.

screen shot 2019-01-21 at 12 31 50 pm

Can't we just update the link to configure for this instance, without changing something that affects anything else?

You could use a form alter but the structure of the array for those menu links gets icky and is easiest to do in the xmlsitemap_menu_entity_info_alter() function like in the attached PR. I tried for a bit to make it work and someone else could finish it on the feature/54-a branch below, but it looks a bit brittle for my tastes to only save the user from an additional click.

https://github.com/backdrop-contrib/xmlsitemap/compare/feature/54-a?expand=1

Unless someone wants to finish that code in a bulletproof way, I'm leaning toward closing this issue in order to not cause regressions in the future due to brittle code.

@laryn laryn added the has pr label Aug 28, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.