Problem/Motivation

It doesn't make sense not to allow to add none for more than one block

Steps to reproduce

1- Edit a block and add the none as ID
2- Edit a second block and also add none as ID

Proposed resolution

Do not check for duplicated ID if the $id_replacement is equal to none

Issue fork block_class-3356441

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

paulocs created an issue. See original summary.

paulocs opened merge request !14

paulocs’s picture

paulocs
Primary language Portuguese, Brazil
Location Belo Horizonte
commented
Status: Active » Needs review
paulocs’s picture

paulocs
Primary language Portuguese, Brazil
Location Belo Horizonte
commented
Category: Task » Bug report
hmendes’s picture

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

Thanks

chris matthews’s picture

chris matthews commented
Parent issue: » #3363340: Plan for 2.0.12 Release of Block Class

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

  • dydave committed ebf3d4ed on 4.0.x authored by paulocs 
    Issue #3356441 by paulocs, dydave: Allow multiple blocks to use '...
dydave’s picture

dydave commented
Status: Reviewed & tested by the community » Fixed

Thank you very much Paulo (@paulocs) for reporting this issue and providing a solution in code, it is greatly appreciated!

Great catch! 👍
Indeed, it doesn't really make sense not to be able to use the value '<none>' for multiple blocks, to allow users to remove the default block ID on multiple blocks. 🤔

After rebasing the MR, I did tests before and after the code changes and was able to confirm they worked as expected:
I was able to save several blocks with the ID value '<none>'.

I checked the display of the blocks and it seemed to work fine as well:
The two blocks would display without any HTML ID.

After updating slightly the inline comment in the merge request, based on the issue summary, I went ahead and merged the changes above at #8. 🥳

I checked in the PHPUNIT tests and currently, the ID field is not tested anywhere....
So that should probably be addressed in a different issue in the future.

For the time being, marking issue as Fixed.

Feel free to let us know if you have any questions or concerns on any of the recent code changes or the project in general, we would be glad to help. 😊
Thanks again very much!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

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