Problem/Motivation

Blocking #2343669: Remove _l() and _url()

Postponed on #2405551: Add a method to support UIs where users enter paths instead of route names and other valid use cases
no.... on #2417333: Add support for user-path: scheme to Url class

There are 4 _l() usages left.
#2404603: Add proper support for Url objects in FieldPluginBase::renderAsLink(), so we can remove EntityInterface::getSystemPath() will remove the renderAsLink (because it is tricky)
This removes the other remaining 3.

because #2417333: Add support for user-path: scheme to Url class use user-path

Proposed resolution

Use \Drupal::l()

Remaining tasks

User interface changes

No.

API changes

No.

CommentFileSizeAuthor
#12 2368653.12.patch2.84 KBYesCT
#1 2368653-url_render-1.patch766 bytespcambra
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

pcambra’s picture

pcambra
he/him
Primary language Spanish
Location Asturies
CreditAttribution: pcambra commented
Status: Active » Needs review
FileSize
2368653-url_render-1.patch766 bytes

Here's a start. Had to use the Url fully qualified name because of the namespace conflict, maybe we should rename Drupal\views\Plugin\views\field\Url to UrlLink or something similar?

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

I would rather say that we should pass along the url object, and the code which generates the URL takes care about that.

Note: This might be both relative and absolute URLs.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs review » Needs work

Based upon my feedback.

YesCT’s picture

YesCT CreditAttribution: YesCT commented
Issue summary: View changes
Parent issue: » #2343669: Remove _l() and _url()
webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Priority: Normal » Critical

This is a hard blocker to a critical issue at #2343669: Remove _l() and _url() so escalating to critical, too.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

Maybe #2405551: Add a method to support UIs where users enter paths instead of route names and other valid use cases is the right thing to use, if we don't know more.

pcambra’s picture

pcambra
he/him
Primary language Spanish
Location Asturies
CreditAttribution: pcambra commented
Status: Needs work » Postponed
Related issues: +#2405551: Add a method to support UIs where users enter paths instead of route names and other valid use cases

Marking as postponed of #2405551: Add a method to support UIs where users enter paths instead of route names and other valid use cases

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Issue tags: +D8 Accelerate NJ

Tentatively tagging. This is a blocker for #2343669: Remove _l() and _url() and is currently blocked by at least two issues, both of which are only major. I'm wondering if based on the discussions that've been had at the sprint if it's possible to un-stick this one somehow, or at least clean-up its dependency chain.

YesCT’s picture

YesCT CreditAttribution: YesCT commented
Issue tags: +blocker
YesCT’s picture

YesCT CreditAttribution: YesCT commented
Issue summary: View changes
dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented 
+++ b/core/modules/views/src/Plugin/views/field/Url.php
@@ -45,7 +45,7 @@ public function buildOptionsForm(&$form, FormStateInterface $form_state) {
     if (!empty($this->options['display_as_link'])) {
-      return _l($this->sanitizeValue($value), $value, array('html' => TRUE));
+      return \Drupal::l($this->sanitizeValue($value), \Drupal\Core\Url::fromUri('base://' . $value, array('html' => TRUE)));
     }

The proper way is user-path:$value now.

YesCT’s picture

YesCT CreditAttribution: YesCT commented
Title: Replace _l in Url::render » Replace _l in all places (3) besides one.
Issue summary: View changes
FileSize
2368653.12.patch2.84 KB

There are 4 _l() usages left.
#2404603: Add proper support for Url objects in FieldPluginBase::renderAsLink(), so we can remove EntityInterface::getSystemPath() will remove the renderAsLink (because it is tricky)
This removes the other remaining 3.

because #2417333: Add support for user-path: scheme to Url class use user-path

still postponed, but on #2417333: Add support for user-path: scheme to Url class

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Postponed » Needs review

Well yeah, but it would be still interesting to see whether things fail.

Status: Needs review » Needs work

The last submitted patch, 12: 2368653.12.patch, failed testing.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented

#2417333: Add support for user-path: scheme to Url class just got committed, so this probably needs adjustment.

Status: Needs work » Needs review

dawehner queued 12: 2368653.12.patch for re-testing.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

I really hope that this just needs a retest.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs review » Reviewed & tested by the community

Awesome!!!!

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

Hell yes!

Committed and pushed to 8.0.x. Thanks!

  • webchick committed f977553 on 8.0.x 
    Issue #2368653 by YesCT, pcambra, dawehner: Replace _l in all places (3...

Status: Fixed » Closed (fixed)

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