API page: https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Link.php/...

Enter a descriptive title (above) relating to public static function Link::createFromRoute, then describe the problem you have found:

In Link::createFromRoutean array with the key of attributes may be added to supply element attributes (class, title, etc). This is missing from the documented description of parameters, in the options section.

CommentFileSizeAuthor
#13 interdiff-2818011-5-13.txt442 bytesrakesh.gectcr
#13 2818011-13.patch1.55 KBrakesh.gectcr
#9 2818011-9.patch574 bytesyashsharma01
#5 interdiff-2-5.txt1.58 KBshashikant_chauhan
#5 2818011-5.patch1.49 KBshashikant_chauhan
#2 2818011-2.patch870 bytesshashikant_chauhan
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

akalata created an issue. See original summary.

shashikant_chauhan’s picture

shashikant_chauhan CreditAttribution: shashikant_chauhan as a volunteer and at Iksula commented
Status: Active » Needs review
FileSize
2818011-2.patch870 bytes

adding patch.

Chi’s picture

Chi CreditAttribution: Chi commented

As this parameter is passed directly to Url component we could just reference the method which accepts it (Url::fromUri) to avoid duplication.

The Url constructor does it this way.
https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Url.php/f...

dalin’s picture

dalin
he/him
Primary language English
Location 🇲🇽, 🇨🇦, 🌍
CreditAttribution: dalin at Advomatic for Annenberg Foundation commented
Status: Needs review » Needs work
Issue tags: +Novice

I agree, let's not duplicate documentation.

shashikant_chauhan’s picture

shashikant_chauhan CreditAttribution: shashikant_chauhan as a volunteer and at Iksula commented
Status: Needs work » Needs review
FileSize
2818011-5.patch1.49 KB
interdiff-2-5.txt1.58 KB

Added suggestion mentioned by Chi.

dalin’s picture

dalin
he/him
Primary language English
Location 🇲🇽, 🇨🇦, 🌍
CreditAttribution: dalin at Advomatic for Annenberg Foundation commented
Status: Needs review » Reviewed & tested by the community
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Needs work

Could we explicitly say the $options param takes exactly the same structure? Otherwise I agree not duplicating is good.

yashsharma01’s picture

yashsharma01 CreditAttribution: yashsharma01 as a volunteer commented
Assigned: Unassigned » yashsharma01
yashsharma01’s picture

yashsharma01 CreditAttribution: yashsharma01 as a volunteer commented
Assigned: yashsharma01 » Unassigned
Status: Needs work » Needs review
FileSize
2818011-9.patch574 bytes

I have added comments and fixed @see documentation changes.

Status: Needs review » Needs work

The last submitted patch, 9: 2818011-9.patch, failed testing.

dalin’s picture

dalin
he/him
Primary language English
Location 🇲🇽, 🇨🇦, 🌍
CreditAttribution: dalin at Advomatic commented

@yashsharma01

  1. I can see where you're going with this, but the patch failed testing because your new patch assumes that the previous patch has already been applied (it hasn't). So you need to submit a patch that starts with the current state of Drupal Core, then makes all the changes that are talked about on this thread. 
  2. -   *   See \Drupal\Core\Url::fromUri() for details.
+   *   The options parameter takes exactly the same structure.

    I think we actually need both of those lines for the sentence to make sense.

rakesh.gectcr’s picture

rakesh.gectcr
he/him
Primary language Malayalam
Location Manchester
CreditAttribution: rakesh.gectcr as a volunteer and at Valuebound commented
Assigned: Unassigned » rakesh.gectcr
rakesh.gectcr’s picture

rakesh.gectcr
he/him
Primary language Malayalam
Location Manchester
CreditAttribution: rakesh.gectcr as a volunteer and at Valuebound commented
FileSize
2818011-13.patch1.55 KB
interdiff-2818011-5-13.txt442 bytes
rakesh.gectcr’s picture

rakesh.gectcr
he/him
Primary language Malayalam
Location Manchester
CreditAttribution: rakesh.gectcr as a volunteer and at Valuebound commented
Status: Needs work » Needs review
Issue tags: +SprintWeekend2017
rakesh.gectcr’s picture

rakesh.gectcr
he/him
Primary language Malayalam
Location Manchester
CreditAttribution: rakesh.gectcr as a volunteer and at Valuebound commented
Assigned: rakesh.gectcr » Unassigned
rosschive’s picture

rosschive CreditAttribution: rosschive commented
Status: Needs review » Reviewed & tested by the community

Taking a look as part of Global Sprint Weekend (Boston)...

Verifying that proposed changes in the interdiff are reflected in @rakesh.gectcr's submitted patch. Additionally, I launched simplytest.me sandbox site (with patch) without incident and site is stable.

webchick’s picture

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

Yay for less Druplication! ;)

Committed and pushed to 8.4.x, cherry-picked to 8.3.x and 8.2.x. Thanks!

  • webchick committed 2572749 on 8.4.x 
    Issue #2818011 by shashikant_chauhan, rakesh.gectcr, yashsharma01, dalin...

  • webchick committed 4b17193 on 8.3.x 
    Issue #2818011 by shashikant_chauhan, rakesh.gectcr, yashsharma01, dalin...

  • webchick committed 0f11204 on 8.2.x 
    Issue #2818011 by shashikant_chauhan, rakesh.gectcr, yashsharma01, dalin...

Status: Fixed » Closed (fixed)

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