Problem/Motivation

Once #2344487: Provide special route names for <current> and <none> is in, replace all usages.

Proposed resolution

In order to do that convert the new path processors to route processors.

Remaining tasks

User interface changes

API changes

CommentFileSizeAuthor
#34 interdiff-2345753-34.txt2.6 KBdamiankloip
#34 2345753-34.patch32.15 KBdamiankloip
#30 interdiff.txt3.49 KBtim.plunkett
#30 2345753-url-30.patch33.06 KBtim.plunkett
#27 interdiff.txt1.93 KBwim leers
#27 2345753-27.patch30.45 KBwim leers
#25 2345753-25.patch30.02 KBwim leers
#23 interdiff.txt1.32 KBwim leers
#23 2345753-23.patch30 KBwim leers
#20 interdiff.txt545 bytesdawehner
#20 2345753-20.patch29.91 KBdawehner
#17 2345753-17.patch30.3 KBdawehner
#14 2345753-14.patch30.29 KBdawehner
#14 interdiff.txt6.39 KBdawehner
#10 interdiff.txt1.29 KBtim.plunkett
#10 2345753-current-none-10.patch17.15 KBtim.plunkett
#7 interdiff.txt1.55 KBtim.plunkett
#7 2345753-current-none-7.patch6.23 KBtim.plunkett
#4 2345753-current-none-4.patch16.16 KBtim.plunkett
#2 interdiff.txt1.52 KBdawehner
#2 2345753-2.patch17.95 KBdawehner
#1 2345753-current-none-1.patch16.43 KBtim.plunkett

Comments

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Postponed » Needs review
StatusFileSize
new 2345753-current-none-1.patch16.43 KB

Hope this works!

dawehner’s picture

dawehner
Primary language German
commented
StatusFileSize
new 2345753-2.patch17.95 KB
new interdiff.txt1.52 KB

Looks great so far, did a couple of more conversions.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Needs work » Needs review
StatusFileSize
new 2345753-current-none-4.patch16.16 KB
tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Needs work » Needs review
StatusFileSize
new 2345753-current-none-7.patch6.23 KB
new interdiff.txt1.55 KB
tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Needs review » Needs work

I finally posted to the wrong issue. I was expecting that to happen all day...

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Assigned: tim.plunkett » dawehner
Status: Needs work » Needs review
StatusFileSize
new 2345753-current-none-10.patch17.15 KB
new interdiff.txt1.29 KB

Well, LinkGenerator's active class handling doesn't work for <current>:

        // @todo System path is deprecated - use the route name and parameters.
        $system_path = $url->getInternalPath();
        // Special case for the front page.
        $variables['options']['attributes']['data-drupal-link-system-path'] = $system_path == '' ? '<front>' : $system_path;

$system_path is the escaped %3Ccurrent%3E

dawehner’s picture

dawehner
Primary language German
commented

One idea could be that we do call toString() but set $options['alias'] = TRUE, which will skip path aliases
but run everything else. Otherwise we could also compare in JS/PHP manually.

dawehner’s picture

dawehner
Primary language German
commented

Trying to use route processors, nice!

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff.txt6.39 KB
new 2345753-14.patch30.29 KB

Let's see.

dawehner’s picture

dawehner
Primary language German
commented
Assigned: dawehner » Unassigned
Issue tags: +Pre-AMS beta sprintbeta target

Let's merge tags

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs work » Needs review
StatusFileSize
new 2345753-17.patch30.3 KB

Let's see.

yukare’s picture

yukare commented

This is not wrong? rage instead of page.

- - { name: service_collector, tag: page_cache_response_policy, call: addPolicy}
+ - { name: service_collector, tag: rage_cache_response_policy, call: addPolicy}

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs work » Needs review
StatusFileSize
new 2345753-20.patch29.91 KB
new interdiff.txt545 bytes

Love it!

xjm’s picture

xjm
she/her
Primary language English
commented
Issue tags: -Pre-AMS beta sprintbeta target +Pre-AMS beta sprint, +beta target
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs work » Needs review
StatusFileSize
new 2345753-23.patch30 KB
new interdiff.txt1.32 KB

This should fix a big chunk of the test failures. Also renamed $new_route to $current_route, which I think makes more sense.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs work » Needs review
StatusFileSize
new 2345753-25.patch30.02 KB

Chasing HEAD.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs work » Needs review
StatusFileSize
new 2345753-27.patch30.45 KB
new interdiff.txt1.93 KB

Be greener!

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Assigned: Unassigned » tim.plunkett

Working on this.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Needs work » Needs review
StatusFileSize
new 2345753-url-30.patch33.06 KB
new interdiff.txt3.49 KB

We're adding a call to processRoute in getPathFromRoute, so the call counts need to be bumped up.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Tim++ :)

dawehner’s picture

dawehner
Primary language German
commented
Assigned: tim.plunkett » Unassigned

+1 in general!

+++ b/core/core.services.yml
@@ -824,14 +824,15 @@ services:
+  route_processor_none:
+    class: Drupal\Core\RouteProcessor\RouteProcessorNone
...
+      - { name: route_processor_outbound, priority: 200 }

I wonder whether we should have the same priorities as before. I just have not thought about it actually

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Needs review » Needs work

Doesn't apply currently, but borrowing parts of this for #2347465: Convert all instances of #type link/links to convert to use routes

damiankloip’s picture

damiankloip commented
Status: Needs work » Needs review
StatusFileSize
new 2345753-34.patch32.15 KB
new interdiff-2345753-34.txt2.6 KB

Needed quite a reroll. Modified the parameters for the outbound route processor,use the route name in the conditional instead, merge the parameters instead of overwriting all of them.

dawehner’s picture

dawehner
Primary language German
commented
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community
Issue tags: -Pre-AMS beta sprint +During-AMS beta sprint

Looks awesome now.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.0.x, thanks!

  • catch committed 4e5c1c2 on 8.0.x 
    Issue #2345753 by tim.plunkett, dawehner, Wim Leers, damiankloip: Remove...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.