This currently derives from EntityConfirmFormBase. EntityDeleteForm might be better, as it might do more for us?

CommentFileSizeAuthor
#3 flagDelete-2617210-3.patch1.59 KBmartin107
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

joachim created an issue. See original summary.

martin107’s picture

martin107 CreditAttribution: martin107 commented
Assigned: Unassigned » martin107

If we do we can remove 3 methods which do not need to deviate from the standard form.

martin107’s picture

martin107 CreditAttribution: martin107 commented
Assigned: martin107 » Unassigned
Status: Active » Needs review
FileSize
flagDelete-2617210-3.patch1.59 KB

I have removed

getConfirmText()
getQuestion()
submitForm()

I have manually test it ... it looks ok.

BUT
If any tests look out for our non-standard old text ... then there will be tears before bedtime.

joachim’s picture

joachim CreditAttribution: joachim commented

> If any tests look out for our non-standard old text ... then there will be tears before bedtime.

Hmm it would seem we don't have any test coverage for flag deletion...

joachim’s picture

joachim CreditAttribution: joachim commented

Thanks for the patch!

Looks good to me.

Tried the patch. The only change I can see is that it says:

> Are you sure you want to delete the flag Foo?

instead of:

> Are you sure you want to delete the Flag Foo?

Which is fine. Core says 'vocabulary' in lower case and without emphasis, and we should switch to doing the same for 'flag'.

Will commit later.

martin107’s picture

martin107 CreditAttribution: martin107 commented

This is of no consequence

BUT looking at getConfirmText()

The other change is "Delete" becomes "Confirm" - which is ok by me.

joachim’s picture

joachim CreditAttribution: joachim commented

The button on the actual form still says 'Delete'.

joachim’s picture

joachim CreditAttribution: joachim commented

Ah, yup: in \Drupal\Core\Entity\EntityDeleteFormTrait:


  /**
   * {@inheritdoc}
   */
  public function getConfirmText() {
    return $this->t('Delete');
  }

  • joachim committed 8ed3c51 on 8.x-4.x authored by martin107 
    Issue #2617210 by martin107: Changed FlagDeleteForm to inherit from...
joachim’s picture

joachim CreditAttribution: joachim commented
Status: Needs review » Fixed

Committed. Thanks again!

Status: Fixed » Closed (fixed)

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