Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί
Account created on 1 October 2010, over 13 years ago
#

Merge Requests

Recent comments

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

Let's see if tests pass or anything needs to be amended there.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

Added tests.

1) In testing with the seven admin theme, the button displays uncentered:

Drupal 9 is end of life while this issue was fixed, so not sure if still a requirement? Being fair I only tested with Claro.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί
πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

penyaskito β†’ created an issue.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί
πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

penyaskito β†’ created an issue.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

#5 worked for me.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

Still an issue with 3.0.2. I still had my changes on lightning_scheduler, need more coffee.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

I was running 3.0.0, this is a non issue with 3.0.2.
Thanks for fixing my bugs before I'm aware they exist.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

penyaskito β†’ created an issue.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

Thanks!

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

LGTM

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

We have a similar issue, and we are NOT using Solr facets, it's entity browsers with views where ajax paging is failing.

We got to debug it to the point we found that it's trying to decompress an already decompressed libraries, so libraries are set to "false".
It's not only about a warning, but ajax is not working as the ajax call produces an error.

I'd say this is Major but "Cause a significant admin- or developer-facing bug with no workaround." it's pretty subjective so leaving it as Normal for now.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί
πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

Still figuring out why for creating a new issue, but we've found that in some edge case TBD, it tries to decompress an already decompressed state, which ends up producing an ajax error because ajax_page_state[libraries] ends up being FALSE.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί
πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

We detected this issue with our project automated tests (Playwright, sadly won't help here) as soon as we updated to ckeditor5 when running for mobile devices (we use Claro).

With the patch at #49 _some_ of our tests are back to green. I still need to figure out what's the case where this is still happening.

I'm surprised that per #50, Seven uses different classes for the draggable tables. Assuming Seven is still supported, what could be a desirable solution here? Is there any other selector we can use that could work for both while this is fixed upstream?

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

Added a check so it's not null. Happened to me on a link like "whatever?argument=" (see = but empty string).

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

penyaskito β†’ created an issue.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

I wasn't aware of that regression, thanks for pointing it!

I've been using the PR patch on prod. Until the core issue is fixed, from an UX perspective I think it's better for end users to have to rethink the menus when they restore content than having to think about removing it from every menu it might be used when deleting something.

So this might serve as a temp solution and we create a follow-up blocked on the core issue?

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

Failures are because of https://www.drupal.org/node/3030340 β†’ needs to be handled.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί
πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

penyaskito β†’ created an issue.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

I think this is ready now. Needs tests though.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

Created a merge request with a solution that worked for us. Javi-er thinks this might be a broader issue and not only affecting tabledrag.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

penyaskito β†’ made their first commit to this issue’s fork.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

#23 didn't work for action submit element, but the MR did.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

Thanks for the feedback!

Definitely needs tests. I've been seeing weird looping issues in our playwright tests, but that might be an issue on the tests or playwright itself.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

Code-wise LGTM.

The only issue we can have with this is altering what people is expecting on their projects. But given we are actually making it deterministic, any expectation they had before this MR is merged is actually wrong and not reliable. So it's definitely not making things worse for them, actually providing a way to fix it via weight if they need to.

So IMHO this is RTBC.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

Fixed another bug when we remove the last item, we want to show the button and the number of pending items again.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

We just needed to process everything no matter if all items were not empty.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

Found an issue, the button is not showing when we are already at the limit of items (e.g. cardinality 3 and I have 3 items).

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί
πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

penyaskito β†’ created an issue.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

D10 release has been working pretty well, so this can be closed.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί
πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

While working on this thought it would be cool to also restore the menu_links if they were also in trash when the related node was restored.

But somehow

    $deletedLinks = [];
    $trashManager->executeInTrashContext('inactive', function () use ($menu_link_manager, $url,$route_parameters, &$deletedLinks) {
      $deletedLinks = $menu_link_manager->loadLinksByRoute($url->getRouteName(), $route_parameters);
    });

didn't work as I expected and $deletedLinks comes empty. I debugged this and it is because `trash_menu_link_content_update` removes the definition, so `menu_tree` doesn't contain the link anymore. We might want to review this and provide this functionality in a follow-up.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί
πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

That looks definitely cleaner. Created MR with proposed solution.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

@amateescu I've never done this before, should it be in ServiceProvider like WorkspacesServiceProvider does?

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί
πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

I'm not asking to skip eslint, I'm asking to ignore compiled files.

Core is using .eslintignore https://git.drupalcode.org/project/drupal/-/blob/11.x/core/.eslintignore...
We should do that and ignore js/build/*

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

This might be a release blocker too.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

Aside of missing tests for this, this LGTM. I'll leave it up to you if you want to add them.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

Created πŸ› If a suggestion has a NULL in query string, LinkitFilter replaces with an empty string Needs review for fixing that on linkit side. Reviews appreciated.
I'm marking this as NR too as #4 works for me.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί
πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

penyaskito β†’ created an issue.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί
πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

This is linting e.g. the build js, which adds tons of noise and makes this impractical.
I'm pretty sure there's some way we can configure that to be ignored before enabling gitlabci.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

The failure was because we had:

    // For media entities, check if standalone URLs are allowed. If not, then
    // strip '/edit' from the end of the canonical URL returned
    // by $entity->toUrl().

If standalone URLs are not allowed, I don't think we should be showing an url that would be a 404. So we actually should show the edit form, which is the canonical url in that case.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

A BC layer was added in https://www.drupal.org/project/linkit/issues/3354873 πŸ› Direct URL to media file entity does not work because relative URL does not pass URL path validation Fixed , but this broke if using the newer APIs. I covered that in the PR, which includes the patch at #3 as the starting point.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

We are using ckeditor4+linkit 6.1.2 and media_entity_download 2.2.0 and we are facing an issue with a duplicated inline (media/2/download?inline?inline) when viewing the edited content. I reproduced this too with ckeditor5, so it's not a ckeditor issue.

The BC code that was added to linkit does:

              $element->setAttribute('href', $url->getGeneratedUrl() . $query . $anchor);

but the generated url already includes ?inline.

If I use #4, then the problem changes to having ?inline= (see equals sign).

I think we should do #4, but still we will need to figure out a follow-up on linkit. Then it would run:

                $parsed_query = [];
                parse_str($href_url['query'], $parsed_query);
                if (!empty($parsed_query)) {
                  $url->setOption('query', $parsed_query);
                }

And that generates ['inline' => ''] instead of ['inline' => NULL].

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

I think the MR at ✨ Translation support Needs review replaces this one.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί
πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί
πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

penyaskito β†’ made their first commit to this issue’s fork.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί
πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί
πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

penyaskito β†’ created an issue.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

penyaskito β†’ created an issue.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

penyaskito β†’ created an issue.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

That's actually why I didn't add a proposed solution and classified as minor.

This happens with any local task. For devel I wouldn't care that much, but if I use entity_clone: do I want to allow to clone trashed content? It should be on a case per case basis. I think we shouldn't care about exceptions for now, and hide all if possible, but if I have the "view deleted entities", tabs will be visible based on permissions.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

penyaskito β†’ created an issue.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί
πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

Unsub. I've not lost interest, but realistically I never can't make it. Keep up the great work!

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί
πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

Thanks for the review!

- the views wizard plugins don't affect node/media items if they don't have trash configured on them, and

I'm worried about what happens if I already added the e.g. nodes view and I disable nodes from the settings page. The field won't exist, so I'm assuming a WSOD. So this probably needs more investigation in its own issue.

- document that these entity types are special-cased in these plugins. In other words, explain somewhere (maybe in a readme, or in the project page?) that if trash is enabled on node/media they will be automatically excluded from new views, but that won't happen with other entity types.

Yes, we will need to document this before releasing a stable version. No docs have been written yet, so I'm fine leaving this unresolved for now.

I'm gonna leave those for a follow-up; πŸ’¬ Views wizards integration Active

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

penyaskito β†’ created an issue.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

penyaskito β†’ created an issue.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί
πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

penyaskito β†’ created an issue.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

We should provide credit also to friera, who worked initially on this as part of the dashboard initiative: https://www.drupal.org/project/3327580/issues/3375851 ✨ Provide a block with DA announcements Needs review

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

penyaskito β†’ made their first commit to this issue’s fork.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί
πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

We have split the bits that require BC layer into follow-ups. The fix itself was RTBC before, and I think it is, so setting it back to RTBC.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

penyaskito β†’ created an issue.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

Fixed!

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

penyaskito β†’ created an issue.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

Created πŸ“Œ Move TextFormat form element from config_translation to text module Active
Created πŸ“Œ Move text_format data type from core to text module Active

I think they are independent of each other. Removing "needs followup" tag.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

penyaskito β†’ created an issue.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

penyaskito β†’ created an issue.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

All feedback has been resolved with @see comments.

Before:
olivero.settings 83%

After:
olivero.settings 100%

I validated that the choices described in the schema constraints are the ones available at the form.
This looks great to me.

A doubt that I have is:
If e.g. I had to alter a form for adding an extra option in olivero settings form in my custom module, I probably need to implement hook_config_schema_info_alter to alter that constraint. I'm not sure if we have already docs/change records including that, but probably this is something worth to document.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

Patch looks good, but doesn't apply anymore.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί
πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

Comitted to 1.0.x, thanks everyone!

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

Needs to define something like

.toolbar-link--dashboard::before {
+  --icon: url("data:image/svg+xml,%3csvg width='24' height='24' viewBox='0 0 24 24' fill='none' xmlns='http://www.w3.org/2000/svg'%3e  %3cpath d='M19.5 3H7.5C7.10218 3 6.72064 3.15804 6.43934 3.43934C6.15804 3.72064 6 4.10218 6 4.5V6H4.5C4.10218 6 3.72064 6.15804 3.43934 6.43934C3.15804 6.72064 3 7.10218 3 7.5V19.5C3 19.8978 3.15804 20.2794 3.43934 20.5607C3.72064 20.842 4.10218 21 4.5 21H16.5C16.8978 21 17.2794 20.842 17.5607 20.5607C17.842 20.2794 18 19.8978 18 19.5V18H19.5C19.8978 18 20.2794 17.842 20.5607 17.5607C20.842 17.2794 21 16.8978 21 16.5V4.5C21 4.10218 20.842 3.72064 20.5607 3.43934C20.2794 3.15804 19.8978 3 19.5 3ZM7.5 4.5H19.5V11.0044L17.9344 9.43875C17.6531 9.15766 17.2717 8.99976 16.8741 8.99976C16.4764 8.99976 16.095 9.15766 15.8137 9.43875L8.75344 16.5H7.5V4.5ZM16.5 19.5H4.5V7.5H6V16.5C6 16.8978 6.15804 17.2794 6.43934 17.5607C6.72064 17.842 7.10218 18 7.5 18H16.5V19.5ZM19.5 16.5H10.875L16.875 10.5L19.5 13.125V16.5ZM11.25 10.5C11.695 10.5 12.13 10.368 12.5 10.1208C12.87 9.87357 13.1584 9.52217 13.3287 9.11104C13.499 8.6999 13.5436 8.2475 13.4568 7.81105C13.37 7.37459 13.1557 6.97368 12.841 6.65901C12.5263 6.34434 12.1254 6.13005 11.689 6.04323C11.2525 5.95642 10.8001 6.00097 10.389 6.17127C9.97783 6.34157 9.62643 6.62996 9.37919 6.99997C9.13196 7.36998 9 7.80499 9 8.25C9 8.84674 9.23705 9.41903 9.65901 9.84099C10.081 10.2629 10.6533 10.5 11.25 10.5ZM11.25 7.5C11.3983 7.5 11.5433 7.54399 11.6667 7.6264C11.79 7.70881 11.8861 7.82594 11.9429 7.96299C11.9997 8.10003 12.0145 8.25083 11.9856 8.39632C11.9567 8.5418 11.8852 8.67544 11.7803 8.78033C11.6754 8.88522 11.5418 8.95665 11.3963 8.98559C11.2508 9.01453 11.1 8.99968 10.963 8.94291C10.8259 8.88614 10.7088 8.79001 10.6264 8.66668C10.544 8.54334 10.5 8.39834 10.5 8.25C10.5 8.05109 10.579 7.86032 10.7197 7.71967C10.8603 7.57902 11.0511 7.5 11.25 7.5Z' fill='currentColor'/%3e%3c/svg%3e");
+}

So might need a css alter?

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

penyaskito β†’ created an issue.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

Feedback is valuable, saving credit.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί
+++ b/config/schema/dashboard.schema.yml
@@ -20,3 +20,18 @@ dashboard.dashboard.*:
+      label: Example

oopsie

Otherwise looks great.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

Pushed to 1.0.x, thanks!

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

Added credit for people that worked on this in Vienna and providing reviews.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

I've tested this and worked as expected.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί
πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

Test look great! Thanks Yan!

+++ b/tests/modules/dashboard_test/config/install/dashboard.dashboard.test.yml
@@ -0,0 +1,38 @@
+dependencies: {  }

I don't think we should change anything here now, but this helped me figuring out πŸ“Œ [PP-1] If a dashboard uses layout builder, add the config dependency Active

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

penyaskito β†’ created an issue.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί
πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

penyaskito β†’ made their first commit to this issue’s fork.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

This patch allows to change between choosing blocks and layout builder integrations by replacing the entity class when layout builder is enabled.

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

Most of this patch was paired between Pablo and me.

This adds a way to select which blocks should be shown without layout builder.
While working on it, we discovered that probably we want to have something more similar to views displays, were you can drag stuff between a content and a disabled region, aside of setting the weights. That would be far more usable than the checkboxes that this patch provides.

Also, we still need to figure out how I would store and configure the block instances themselves, which is something that is completly ignored by this patch.

Also, we are rendering the blocks, but we still need to figure out the instance values, and also decide how we render title of the block et al (which might be related with the previous pending task)

πŸ‡ͺπŸ‡ΈSpain penyaskito Seville πŸ’ƒ, Spain πŸ‡ͺπŸ‡Έ, UTC+2 πŸ‡ͺπŸ‡Ί

@Sahana_N We want to use views if possible. Thanks for planning to work on this!

Production build https://api.contrib.social 0.61.6-2-g546bc20