Problem/Motivation

In #3252386: Use PHP attributes instead of doctrine annotations we added support for attribute based plugin discovery.
As part of that issue we converted block and action plugins.

This issue is to convert \Drupal\views\Annotation\ViewsStyleplugins to use Attributes.

To do that we need #3421008: Convert ViewsExposedForm plugin discovery to attributes first. This issue is postponed on that.

Proposed resolution

  1. Add a class to represent the new Attribute - Example
  2. Update the plugin manager constructor to include both the attribute and annotation class names - example
  3. Convert all plugins that use the annotation to use the new attribute - example

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#11 3421005-nr-bot.txt90 bytesneeds-review-queue-bot

Issue fork drupal-3421005

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

larowlan created an issue. See original summary.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Issue summary: View changes
Related issues: -#3420998: Convert ViewsPluginAnnotationBase plugin discovery to attributes +#3421006: Convert ViewsDisplayExtender plugin discovery to attributes
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Issue summary: View changes
Related issues: -#3421006: Convert ViewsDisplayExtender plugin discovery to attributes +#3421008: Convert ViewsExposedForm plugin discovery to attributes
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Title: [PP-1] Convert ViewsStyle plugin discovery to attributes » Convert ViewsStyle plugin discovery to attributes
Status: Postponed » Active

sorlov made their first commit to this issue’s fork.

sorlov opened merge request !7046

Ruturaj Chaubey made their first commit to this issue’s fork.

sorlov’s picture

sorlov commented
Status: Active » Needs review
sorlov’s picture

sorlov commented
Status: Needs review » Needs work
ruturaj chaubey’s picture

ruturaj chaubey
He/Him
Primary language English
Location Pune, India
commented
Status: Needs work » Needs review
needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 3421005-nr-bot.txt90 bytes

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

sorlov’s picture

sorlov commented
Status: Needs work » Needs review
smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

Appears to have a test failure, may need a rebase

quietone made their first commit to this issue’s fork.

quietone’s picture

quietone commented
Status: Needs work » Needs review

I've added register_theme: FALSE, to the Serializer plugin because it does not define a theme.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs Review Queue Initiative

As before great investigative work @quietone

Searched the repo and all 14 instances of @ViewsStyle have been replaced.

Rest of changes seem fine to me.

smustgrave’s picture

smustgrave commented

Added some changes based on feedback from @alexpott on https://git.drupalcode.org/project/drupal/-/merge_requests/7035#note_287513

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work

Added some comments to the MR.

smustgrave’s picture

smustgrave commented
Status: Needs work » Reviewed & tested by the community

Okay I fixed this one since I broke it before.

Applied the changes. Since they were minor going to remark RTBC.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work

Added some comments to the MR.

quietone’s picture

quietone commented
Status: Needs work » Needs review

Applied the suggestions from @alexpott and rebased.

smustgrave’s picture

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

Feedback appears to be addressed.

Sorry for breaking it before glad to see it wasn't hard to fix :)

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Version: 11.x-dev » 10.3.x-dev
Status: Reviewed & tested by the community » Fixed

Committed and pushed f944f4886c to 11.x and a28538b0ab to 10.3.x. Thanks!

  • alexpott committed a28538b0 on 10.3.x 
    Issue #3421005 by Ruturaj Chaubey, sorlov, quietone, smustgrave,...

  • alexpott committed f944f488 on 11.x 
    Issue #3421005 by Ruturaj Chaubey, sorlov, quietone, smustgrave,...

quietone closed merge request !7046

Status: Fixed » Closed (fixed)

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