backdrop-contrib / examples

Examples for Developers
GNU General Public License v2.0
7 stars 9 forks source link

Page Example not working #105

Open stpaultim opened 2 months ago

stpaultim commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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

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 commented 2 months ago

@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 commented 2 months ago

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

avpaderno commented 2 months ago

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 commented 2 months ago

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

avpaderno commented 2 months ago

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

stpaultim commented 2 months ago

@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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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

stpaultim commented 2 months ago

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 commented 2 months ago

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

stpaultim commented 2 months ago

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