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

Comments

akalata created an issue. See original summary.

shashikant_chauhan’s picture

shashikant_chauhan commented
Status: Active » Needs review
StatusFileSize
new 2818011-2.patch870 bytes

adding patch.

chi’s picture

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 🇨🇦, 🇲🇽, 🌍
commented
Status: Needs review » Needs work
Issue tags: +Novice

I agree, let's not duplicate documentation.

shashikant_chauhan’s picture

shashikant_chauhan commented
Status: Needs work » Needs review
StatusFileSize
new 2818011-5.patch1.49 KB
new interdiff-2-5.txt1.58 KB

Added suggestion mentioned by Chi.

dalin’s picture

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

catch
he/him
Primary language English
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 commented
Assigned: Unassigned » yashsharma01
yashsharma01’s picture

yashsharma01 commented
Assigned: yashsharma01 » Unassigned
Status: Needs work » Needs review
StatusFileSize
new 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 🇨🇦, 🇲🇽, 🌍
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 English
Location Manchester
commented
Assigned: Unassigned » rakesh.gectcr
rakesh.gectcr’s picture

rakesh.gectcr
he/him
Primary language English
Location Manchester
commented
StatusFileSize
new 2818011-13.patch1.55 KB
new interdiff-2818011-5-13.txt442 bytes
rakesh.gectcr’s picture

rakesh.gectcr
he/him
Primary language English
Location Manchester
commented
Status: Needs work » Needs review
Issue tags: +SprintWeekend2017
rakesh.gectcr’s picture

rakesh.gectcr
he/him
Primary language English
Location Manchester
commented
Assigned: rakesh.gectcr » Unassigned
rosschive’s picture

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 🇨🇦
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.