Changing link title configuration leads to displaying old values

Not sure about the nested ternary, but that's just a preference and in this case moving it out of the array doesn't necessarily help. Otherwise looks good.

1. +++ b/core/modules/link/src/Plugin/Field/FieldWidget/LinkWidget.php
   @@ -243,14 +243,15 @@ public function formElement(FieldItemListInterface $items, $delta, array $elemen
   +    $title_settings = $this->getFieldSetting('title');
   

   a nitpick so small you need an electron microscope to see it, but I think this should be $title_setting, not $title_settings

2. +++ b/core/modules/link/src/Plugin/Field/FieldWidget/LinkWidget.php
   @@ -243,14 +243,15 @@ public function formElement(FieldItemListInterface $items, $delta, array $elemen
   +      '#default_value' => $title_settings === DRUPAL_DISABLED ? NULL : ($items[$delta]->title ?? NULL),
   

   #26.1 still hasn't been addressed here, and was also flagged by @catch as iffy

Addressed #45.1

Needs work for #45.2

@Rinku Jacob 13, Before testing an issue it is good practice to read the comments. The work requested in #45 is not complete which means this patch is not ready for testing.

Hi @quietone , sorry for the mistake i have done. I didn't noticed comment on 45.2. according to comment on 26.1 we can change '#default_value' => $title_setting === DRUPAL_DISABLED ? NULL : ($items[$delta]->title ?? NULL), to'#default_value' => $title_setting === DRUPAL_DISABLED ? NULL : ($items[$delta]->title), because it shouldn't raise and error. I think this changes we need to do here. If it's wrong please excuse me. I f it is correct i will create a patch based on that

#46 seemed to be making additional changes then 45.1

Addressed both points. Just moved the ternary check above to make it easier to read.

This issue is being reviewed by the kind folks in Slack → , #need-reveiw-queue. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request → as a guide.

Manually tested the patch on 10.1.x-dev and it resolves the issue. The IS looks good. Before/after screenshots for this iteration were added in #52.

+    $this->assertSession()->pageTextContains('entity_test ' . $id . ' has been updated.');

I wondered if this should use FormattableMarkup, but given that \d+ matches only integer values, I think it can be ommitted in this case. Didn't find any other issues. So I'd say the code looks good.

Bottom line: I think the issue is RTBC.

The last submitted patch, 50: 3165999-50.patch, failed testing. View results →

Random failure

The last submitted patch, 50: 3165999-50.patch, failed testing. View results →

Random

The last submitted patch, 50: 3165999-50.patch, failed testing. View results →

Think it's random.

The last submitted patch, 50: 3165999-50.patch, failed testing. View results →

Random failure

Ahmad Smhan → made their first commit to this issue’s fork.

+++ b/core/modules/link/src/Plugin/Field/FieldWidget/LinkWidget.php
@@ -244,14 +244,19 @@ public function formElement(FieldItemListInterface $items, $delta, array $elemen
+    if ($title_setting !== DRUPAL_DISABLED && isset($items[$delta]) && isset($items[$delta]->title)) {

ubernit: do we need two calls to isset here?

I think php will allow just the second one

Fine to self-rtbc after that change, please ping me to look again at that point.

Hi @larowlan,
I have removed the isset($items[$delta]) as per #63 comment.

Thanks!

Looks like failing linting, thanks

Updated patch #64 by fixing it, please review it.

Thanks!

Removing credit for Ahmad Smhan for the empty commit

#64 appeared to be adding out of scope of changes

#66 seems good. Since this was previously RTBC and nitpicky change, think safe to remark it.

Updating issue credits

I was about to commit this and whilst going through my internal checklist realised that we're making a widget/form level change here but we're not doing anything to address API level changes too.

Which leads me to believe that the fix probably belongs in LinkItem instead of in the widget.

So can we undo our changes to the widget here and instead change LinkItem::setValue to set the title to NULL when the title option is set to Disabled

We can use \Drupal\Tests\link\Kernel\LinkItemTest::testLinkItem to extend test coverage for that change.

We can retain the test changes we've made in this issue, but we need to undo the changes to the widget.

Sorry for not realising this sooner, I realise we've been around and around a few times on this one.