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

Page Example not working #105

Open
stpaultim opened this issue Jun 21, 2024 · 14 comments · May be fixed by #107
Open

Page Example not working #105

stpaultim opened this issue Jun 21, 2024 · 14 comments · May be fixed by #107

Comments

@stpaultim
Copy link
Member

I'm a bit out of my depth here, but maybe if I try to fix some problems in this module. I can learn a few things.

I was trying to use this to learn some basic stuff about menus and pages. I tried to the use the Page Example module and no menu item shows up.

I'm going to try and submit a PR to fix it.

@stpaultim stpaultim linked a pull request Jun 21, 2024 that will close this issue
@stpaultim
Copy link
Member Author

So, I've submitted a PR that adds the example menu item to the "main-menu".

I'm not entirely sure that is the best place for it. But, I thought I would give that a try and get some feedback. Maybe I can learn something, while improving this module.

I think that this module could be REALLY useful to folks like me, but right now it's still a bit of a mess.

@argiepiano
Copy link

argiepiano commented Jun 21, 2024

Hi @stpaultim. Thanks for your interest in these Examples. The approach to menu links is different between D7 and Backdrop

  • In D7, all menu items that don't have a key 'menu_name' were placed in the default menu called navigation
  • In Backdrop, those menu items are placed in a hidden menu called internal. That menu is not supposed to be viewable, and it doesn't appear in the menu UI

So, this needs to be documented in the spot you modified. I would advice against modifying the line you changed with 'menu_name'. Your PR puts it in the main-menu. Instead I'd suggest leaving it as is, and changing the documentation above, and perhaps adding another menu item that does make use of the key 'menu_name'

@stpaultim
Copy link
Member Author

stpaultim commented Jun 21, 2024

@argiepiano Thanks for all the work you do at helping folks like me understand this stuff better.

Your explanation of the difference between how Drupal 7 and Backdrop CMS handle these links is helpful. I was going to argue a bit with you about your suggested solution, but now that I think about it your solution makes good sense.

One example (current example) that uses the internal menu only and is not visible.
A new example that makes it visible in the main-menu.

I like that. I'll try to update my PR. It doesn't make sense to me that I can't find these links at all, but having both options is probably better for the purpose of educating folks - along with some updates to the documentation.

On occasion I come to the examples module for help when trying to figure stuff out. Sometimes it's helpful, but sometimes little differences like this one frustrate me and it's just not helpful. With a little work, this could be a much better resource.

I'll try to help out when I can.

@avpaderno
Copy link
Member

Yes, that comment says what happens in Drupal 7. Backdrop does not put them in the Navigation menu anymore, as @argiepiano said.

@avpaderno
Copy link
Member

As for the menu not showing up, isn't this a problem with all the submodules?

The Page Example module uses examples/page_example as route; the Batch Example module uses examples/batch_example. I should see an Examples menu where all those examples/* sub-menus are shown. In this way, I could navigate between all the examples.

@avpaderno
Copy link
Member

(I would prefer a page with links for each reachable page created by each submodule, but that could be my personal preference.)

@avpaderno
Copy link
Member

(As a side note: The forked repository is 128 commits behind the main repository.)

@stpaultim
Copy link
Member Author

@argiepiano and @kiamlaluno

I updated my PR, I left the first link hidden in the internal menu. I then added a new link that would be visible, see screenshot below.

Still not sure of link titles and location of this link. @kiamlaluno suggested NOT showing this link in existing menu, but then where do we expose it for people to find and learn from. Previously, none of the links in this sub module were visible and it was very confusing for me.

I have not experimented enough with other submodules to see if there is a single consistent solution that can be repeated over and over again.

image

image

@argiepiano
Copy link

Since this example tries to illustrate menu routes and menu links, I think it's a good idea to show the link in the main menu. Looks good to me.

@avpaderno
Copy link
Member

avpaderno commented Jun 23, 2024

For the Page Example sub-module, it makes sense and it is good. For the rest of the sub-modules, which should be reachable in some way, a different way must be found.

(Since this issue is just for a single sub-module, it does not need to do much more.)

@avpaderno
Copy link
Member

avpaderno commented Jun 23, 2024

Eventually, I would make clear from the title that is a menu item added by an example sub-module.

@stpaultim
Copy link
Member Author

Eventually, I would make clear from the title that is a menu item added by an example sub-module.

I welcome ideas and will think about some better ways to do this. But, I also think that these menu items will appear only when someone is experimenting with the examples module and it should be pretty clear why they are there.

Having said that, I'm always in favor of better labels or making things easier to understand, if anyone has specific suggestions.

@avpaderno
Copy link
Member

The currently used title is Page Example. The menu title should be that, instead of Page Menu Visible.

@stpaultim
Copy link
Member Author

@kiamlaluno Thanks for your feedback on PR. Will try to adjust ASAP.

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 a pull request may close this issue.

3 participants