Closed (fixed)
Project:
Metatag
Version:
2.1.x-dev
Component:
Code
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Reporter:
Created:
13 Mar 2018 at 20:13 UTC
Updated:
16 Nov 2024 at 11:09 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
marysmech commentedWe needed this too so we implemented it :-) Please see attached patch.
Patch adds support for contextual arguments. So if you have route like this
You can add (using "Add meta tag for custom route" button) URL for example
/node/1/gallery/node/2/gallery/node/3/galleryand for each address you can define specific metatags. Everything is still exportable/importable and should be compatible with previous version of this module.
Comment #3
marysmech commentedComment #4
waldomero commentedHi @marysmech. Thanks for your excellent contribution
I have tested your patch and I think it works fine but I didn't find a way to set a default metatag for the route without an specific parameter, for example, if you want to set a metatag for all the routes like
/node/{id}/gallery.I think that the users would like to have the posibility of set some default metatags for a route and to customize some routes with certain parameters. I think it would be excellent if an user, for example, could set the tag title: "Title for gallery" for all the
/node/{id}/galleryroutes and also he could set the tag title: "Title for gallery one" for the specific route/node/1/gallery.What do you think?
Comment #5
zaporylieThis is very interesting issue. The main issue with patch in #2 is not allowing for wildcard-style route config as suggested in comment #4.
Let's take an example mentioned above and elaborate on it a bit:
/node/{node} <- this is canonical entity route
/node/{node}/gallery <- custom route displaying content of Media reference field stored on Node
/node/{node}/gallery/{media} <- custom route displaying single Media item
As of #4 let's say we want to customize metatags on per-media item level. In case we use current approach suggested in #2 we would have to create number of config entities - one per media entity in node, ex:
/node/1/gallery/1
/node/2/gallery/2
/node/2/gallery/3
/node/3/gallery/2 <- this is the same Media entity as in /node/2/gallery/2 but living in the context of Node 3
This solution is very explicit, it allows us to do all kinds of crazy metatag customizations but it just doesn't scale. If we have 10, 100, 1000, etc. media entities we will bloat the system with endless config entities.
That's why we should allow for wildcards in the path, ex.
/node/*/gallery/* (1)
/node/2/gallery/* (2)
/node/*/gallery/3 (3)
/node/3/gallery/2 (4)
In this scenario we have 4 patterns, where all, one or none param is transformed into wildcard.
Now, for the purpose od this discussion, let's assume we've declared all 4 paths via metatag_routes module. What would be the order in which they execute? For me the natural order would be from the most to the least specific - so in case Node ID is 3 and Media ID is 2, metatag number {4} will be used. If Node ID is 1 and Media ID is 1 the metatag config number (1) will be used. The problem becomes more tricky when Node ID is 2 and Media ID is 3 - which one of two (2) and (3) should take precedence? Which one is more specific?
Perhaps the easiest way of solving it before we decide on strategy (which I imagine will take forever) would be to sort matching config metadata (as collected in metatag_routes_metatags_alter) by weight? Disclaimer: I haven't looked in whether it's possible to add new property. For now we could give all of them weight 0. If the weight of two matching metetag entities is the same we will use sorting strategy we agree upon in a followup issue.
The second issue would be - how do we pass url param context to metatag? The natural way would be using tokens - metatag module use them already for content entities. Unfortunately only one entity can be passed as token replacement context which is an issue considering use case ilustrated above (both Node and Media entity passed as context). Furthermore why limiting to entities? Param can be string or any value, including objects, provided by ParamConverter. Perhaps we could explore global level tokens and pull the values from url similarly to `[current-page:url:args:value:?]`?
Comment #6
smavri commentedHere is a the correct patch after updating to 8.x-1.1 version.
Comment #7
id.conky commentedPatch compatibility fix after upgrate to 8.x-1.2.
Comment #8
id.conky commentedPatch re-upload.
Comment #9
judapriestThanks for the work, and thanks the patch. Reroll on #8 seems to work.
Don't hesitate to make an interdiff. Easier to review ;).
Comment #10
musa.thomashello just try the #8 patch but still a problem, you can try with path argument in letter like /test/{wild}=> /test/toto
The problem come from the hash is differenet of the route generate and the config saved.
Comment #11
musa.thomashere a quick patch start from #8 I just replace the has with array_keys
Comment #12
johnsiciliThe last patch failed to apply for me on 8.x-1.3.
My two cents/use-case on this is to use one meta title and description for every variation.
Furthermore, I need to have the slugs (I have two in my URL) available as a token and with the ability to modify them (e.g. replace dashes with spaces, capitalize, etc.).
Maybe it's possible to have a callback in the modules route? I'd be good with defining metas there and have PHP to help out.
BTW, hi @Jason.bell! Hope you've been well.
Comment #13
anybodyThis projects functionality is now part of Metatags. Please see #3295162: Deprecate Metatag Routes.
Comment #14
damienmckennaLet's move this to the Metatag issue queue. The patch will need a reroll.
Comment #15
justskew commentedPatch for 8.x-1.22
Comment #16
lamp5Comment #17
alvar0hurtad0Please find the attached patch.
Comment #18
alvar0hurtad0I've been testing the patch in real life and I found an issue, this new patch prevents some issues in when you need to save several paths for same controller.
Comment #19
biancaradu27 commentedComment #20
biancaradu27 commentedComment #21
biancaradu27 commentedComment #22
biancaradu27 commentedComment #23
damienmckennaLet's try to resolve this for the upcoming 2.1.0 release.
The patch needs to be rerolled against the 2.0.x branch.
Comment #24
anybody@alvar0hurtad0 or @biancaradu27 could you perhaps reroll this against 2.0.x or even better turn this into a MR against it?
Based on the results we should eventually consider this for 2.2.0 to not block 2.1.0 here? Seems it's not that important for the community (not much activity)?
Comment #27
loze commentedI created a MR with the latest patch
This still doesn't address using wildcards in paths, see #5
Comment #28
loze commentedComment #29
anybodyThanks! Tests are failing. NW!
Comment #30
damienmckennaI reran the tests because the failure was unrelated to this issue.
Comment #31
damienmckennaThis is a separate issue to deal with the test that failed: #3464483: Fix MaintenanceModeTest test failures
Comment #32
damienmckennaThe MR needs to be updated to work off the 2.1.x branch.
Comment #33
anybodySeems like I have no permission to change the target branch. Can you do that @damienmckenna?
Comment #34
damienmckennaComment #35
damienmckennaComment #37
damienmckennaCommitted. Thanks everyone!