Make block for slot selectable via select form widget

I am not sure if this is enough. There are quite some possiblities to introduce new slot id's. For example via views or even via twig.
I have a concept idea to deal with this if you'd like to talk about it. I mailed you my contact data :)

See my issue fork changes. Please review and give feedback.

I modified the form element type from machine name to select and provided the slot configs as options. At least the selection part in the custom block content does work now:

After I do save the block content, the placeholder text for the slot still appears:

Maybe anyone of the maintainers can check, why the block display via the selected slot does not work?

Thats because slots does not work that way. You need to enter the slot_id as a condition value, not the block id :)

You need to enter the slot_id as a condition value

Thanks for the hint! 👍 I've improved the code within the issue fork and now it works. You can select the slot block via its readable block id:

After saving the block content, the content will appear in the slot:

See the related code changes here: https://git.drupalcode.org/issue/slots-3495680/-/commit/440778a95942f877...

I mailed you my contact data :)

I have not received anything, yet. Checked the contact function on d.o for my correct e-mail address and took a look into my spambox. I'll email you my e-mail address.

I set it to your gmail. I am also reachable at slack if you are around there.

We talked about a potential solution via tracking calls of the SlotsService::buildSlot method, which might be the only common call in the different slot integrations. A first easy storage for that could be the key_value store.

Thx again for the exchange. 👍 I've introduced the key-value store. The SlotsService::buildSlot() didn't brought up all slots, which are rendered on one particular page. Therefor the Slots module is now collecting the slot ids via SlotBlock::build() method. Please review the changes.

Hey great work here! I am not fully convinced that the buildSlot method is not suitable, maybe a cache problem? But for now let's work with that.
Can you open a MR so I can add inline feedback?
Here what I can already tell you:
- The new storage service needs an interface
- The condition plugin can do dependency injection by implementing ContainerFactoryPluginInterface
- The default slot_id configuration in the condition is defined in defaultConfiguration, if we need it to be NULL now, just adjust the value there

Hi hydra! I've done the refactoring, based on your feedback. There's also a merge request now. Please check.

Thanks for your feedback! I did the requested changes. Your mentioned docblock comments and newline stuffs etc. is fixed. I also replied to your questions.

The phpcs stuff could be handled in https://www.drupal.org/project/slots/issues/3496183 ✨ Static code analysis Active so it could be just pointed by the CI pipeline. I run PHPCS against the module and there are already lots of other violations against the Drupal coding standard. Most of them could be fixed automatically. So I executed phpcbf to fix them, too.

The comments I wrote manually. However, I am not a friend of this docblock comments. I am more for good variable names, readable and also validated code. Doc comments can easily get legacy and misleading over time.

Thx, it's like I mentioned in your other issue. We won't fix the phpcs issues in this branch anymore. A new version is coming with a new UI and fixed coding styles :)

> We won't fix the phpcs issues in this branch anymore. A new version is coming with a new UI and fixed coding styles :)

I would like to see code style issues handled via pipeline. There are not many violations which need manual coding, so this could be resolved relatively quick. Drafted a MR for this in #3496183 (Static code analysis) → . So you could merge the pipeline config into your branch with the UI, if you like. However, I am pausing work on this issue for now. Just let me know, when it's reasonable to continue with this.

Do you like the current code changes, regarding the select form widget? Would be nice, if we could close this issue.

Yeah, I merged it and will take the new functionality over to the new branch. Thx!

peter majmesku → closed merge request !35

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