Mechelen, 🇧🇪
Account created on 22 November 2012, over 12 years ago
#

Merge Requests

More

Recent comments

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

The latest version of the patch is to disable the option in the views UI, but the issue summary doesn't yet reflect that. So that would be the first thing to do here. As a next step we should transform the patch into a merge request.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

Actually I think this can go back to rtbc

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

ExtentionAvailable makes a lot of sense to me.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

Back to rtbc now that it is moved.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

While I agree with #5 there would not be a big maintenance burden, in the 5 years since this comment was posted there did not seem to be a big interest by others. I would live to see popularity numbers from this based on a contrib module and we could use that to inform the need for this, because to me it still seems like a super small usecase.

I will close this as won't fix for now.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

The change record that is linked to this issue is not correct it seems?

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

I too am constantly bouncing between the need to validate everything strictly and the looseness that is allowed currently.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

Can we do this the same way we define plugins? Each one having their own schema?

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

I reviewed the CR. It has all the information needed.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

I have some small documentation remarks, but this looks great.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

Still needs tests, the remark in #11 seems to be fixed.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

Olivero was specifically designed like this, see 📌 Olivero: Center align content (instead of left align) Needs work for people who want to have a centered layout. But the drops background + left alignment is a feature, not a bug.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

There are more tests that are failing on the last test run. All in ChainedFastBackend.

The change however makes sense and it has good test coverage.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

The parent issue was committed.

Since it's about removing a role, I'm not sure if we need to have strict validation.
However it could lead to an action that doesn't do anything when it has a role configured that does not exist I guess.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

This looks great and it makes this test a bit faster.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

Removing the tag, title looks great now.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

Thanks for that information. Moving to rtbc based on #10.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

Add belgium

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

There's a test failure in AssetAggregationAcrossPagesTest, but I've seen that fail in a couple other merge requests today. I'm thinking it is not related. The only bytes we are saving are from the comment not being there anymore.
I agree keeping the class around though, otherwise this would be a breaking change I think.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

It is green now, and we're decreasing the page weight, that's great.
I am not sure about moving the file though, people are not supposed to reference the file by path, but it seems like it increases the likelihood this will be a breaking change?

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

Removing tags that are no longer relevant.
The new solution is well documented, but I'm not sure about the tradeoffs here, tagged for product manager review because of that.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

I don't see why that test would start failing, and I can't find an issue about it having random failures. Since this a security hardening I think we should move this through.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

So, at the very least this issue sounds premature until the Drupal CMS being stable with a good search_api recipe is implemented. At that point, it sounds like the only concern is around user confusion?

This is where we are now, Drupal CMS 1.0 was released and it includes Search API with a good setup.
I'm not sure how much continued adaption it requires from people building on top of Drupal CMS, but I hope it is minimal.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

I'm not sure about this. Administer Search is from the core search module. Search API doesn't have that permission, but it does have a Search index view mode as well.

How would we, as search API maintainers, allow this to work without asking people to enable core search, just to get that permission.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

These changes look good. It also has the change to phpcs.xml.dist to prevent regressions.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

All threads seem to be complete. Looks good to me.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

I agree with catch in #19, moving this to Drupal\Core\Config seems like a better solution, let's do that.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

I don't understand having to bypass NotNullConstraint? I think we have to reimplement it in here?

We should probably start with a reroll here.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

This needs another rebase.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

I have gotten in the habit of using the markdown-like backticks when having to say that something is `code`. I think a lot of people will recognize this. Is that an option instead of using quotes?

We could even update the api.do parser to wrap those in a code-block as a next step?

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

Since this is only for the ajax behavior of the regular blocks, and we are currently only supporting ajax-like behavior through facets 3.x, I'm going to close this as won't fix.

The current suggestion is to try using views' ajax handling with facets 3.x for this.

It could be that this is also a bug on core views, but that would need a core issue to fix.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

Only 2 instances, those look good. Back to rtbc.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

I am not sure why this was suggested, but I don't think we need to check this? Any module could provide a plugin?

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

While I agree that adding more functional tests is not great for the total test time duration, having them well documented is important.
We can always change the documentation when we want to add more testing to this.

To me, this change seems like it answers the question by @nod_.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

Test looks solid, I think that's good. I wonder if we need a change record for this, but I don't think we do. RTBC++

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

This should be a merge request, not a patch?

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

I agree, this now no longer decreases readability, I think it does read better now.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

This looks great and it has the phpcs change need to ensure we don't regress.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

Found some places where we could align better, I personally really dislike things that end up looking like this: ]));

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

I'm not sure how facets should support this? As long as Search API gives us this as a string we can work with it. So I guess this is supposed to be in the realname project to support Search API

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

Last but not least: per @lauriii, there won't be a "search" or "search index" view mode for XB — search must be configured to use the "default" view mode's representation of content entities both for 1) crafting a search index to power the search, 2) presenting search results. Related: 🌱 [META] Support alternative renderings of prop data added for the 'full' view mode such as for search indexing or newsletters Active and 📌 Create a configurable search api processor for XB data Active

It's not clear to me yet how that avoids the

but without elements that would 'pollute' the search results like field labels, related content blocks, CTAs, social widgets etc. so that if I search for "newsletter" or "Facebook" I don't get every article on the site back in the results, but only the ones with content actually mentioning newsletters or Facebook.

problem described in 🌱 [META] Support alternative renderings of prop data added for the 'full' view mode such as for search indexing or newsletters Active though.

Rendering Search results in the same way as the content's detail page doe not seem like the most commonly chosen way of configuring it. I did a quick poll at the contribution room at Drupal Mountain Camp, and everyone who answered me always configures some kind of teaser, card or search-result view mode when rendering a search result page.

For the indexation part, I also don't see a way that would remove those pollutions without having a seperate view mode, but I personally think that's not as bad. When needing complete control over what all is indexed, it should be indexed using fields, because then it's possible to boost per-field. It gives a lot more control.

This however would mean something like https://www.drupal.org/project/layoutbuilder_search_api for all components/props, which is another approach that could be taken. This removes the double rendering that catch talked about in #13.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

This is however not very precise, if you have a common footer component on every page those elements end up in the index for every page.

We are using the view builder for an entity to run this, so footers/header will not be included directly, this should only render the main content, so that's only the case of a call to view() also includes a header and footer.
https://git.drupalcode.org/project/search_api/-/blob/8.x-1.x/src/Plugin/...

The proposed solution should provide a list of enabled component config objects and a checkbox for each of their props.
For example, you might have a card component with fields for image, title and teaser text.

This means that for every new component I create, I need to configure this processor? Or will it include new ones by default? Why would we do this at the Search API level and not on a new view mode?

As one of the people who's written parts of Search API's backend interface, I wonder how well that UI will work for the target audience of drupal cms. Search API is very powerful, but that power also gives you quite some configuration options. I don't think the target audience of experience builder will find joy in using our UI.

I also wonder how this will work with react components that are created through the ui, will we have to do some kind of parsing to get their data out? What if it uses some kind of API or Javascript service to create content, will those we have to render the javascript from the backend?

I think instead of creating a new processor, we might have to create a new DataSource instead? That datasource could do the specific rendering/fetching of data without it having to go in a processor with it's own form.

I think supporting this through an alternative view mode, is probably the better way to go instead of using a processor like this?

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

Can someone explain to me how to run this gitlab job? I was looking at this with @vrancje and we couldn't see it right away. This should also be documented

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

CR that's added looks great and has all the correct info, RTBC+1

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

Facets 3

Facets 3.x only supports Search API, and introduces "Facets exposed filters" next to "Facet blocks".
We recommend using Facets exposed filters for new projects, when using Facets + Views
Using "Facets exposed filters" in Facets 3 has full AJAX support.

Facets 3 will not support AJAX for "Facet blocks".

Facets 2

Facets 2 works with Drupal Core Search and Search API.
It is kept available for existing projects using Facets 2 + Ajax as there is no upgrade path for this setup. Updating from 2 to 3 without AJAX support is possible.

I think this is very clear, for facets 2 you need the additional core search module as mentioned here, if this doesn't work I think we should file a bug report there. Readme update in #3506177

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

Since this is in the 2.x version of facets and it is regarding ajax, I'm going to close this issue. We are only supporting ajax with views in facets 3.x.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

Since this is in the 2.x version of facets and it is regarding ajax, I'm going to close this issue. We are only supporting ajax with views in facets 3.x.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

I can't reproduce this.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

It no longer does out of the box, you can find the old code in the facets_core_search module, but this is not actively maintained. We however suggest you use Search API instead.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

Since this is in the 1.x version of facets and it is regarding ajax, I'm going to close this issue. We are only supporting ajax with views in facets 3.x.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

I don't really understand the issue here. Can you try again on the latest version and if it is still an issue can you add a screenshot of the issue?

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

Since this is in the 1.x version of facets and it is regarding ajax, I'm going to close this issue. We are only supporting ajax with views in facets 3.x.

Thanks for reporting this issue, I hope it no longer applies on 3.x, if it does we can reopen this issue and target 3.x

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

Tagging as novice

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

I have tried to figure out if we are still missing something, but to me it seems like this is complete.

I was wondering if this would break styling on existing websites, but it won't, because the ID would already change when using ajax.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

I reviewed the issue together with @wannesdr at drupal mountain camp. I had some remarks, which I shared in person. Wannes is fixing those as we speak. I think this issue looks good and I will rtbc this when the last things are fixed.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

Since this is in the 2.x version of facets and it is regarding ajax, I'm going to close this issue. We are only supporting ajax with views in facets 3.x.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

Since this is in the 2.x version of facets and it is regarding ajax, I'm going to close this issue. We are only supporting ajax with views in facets 3.x.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

Since this is in the 2.x version of facets and it is regarding ajax, I'm going to close this issue. We are only supporting ajax with views in facets 3.x.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

Since this is in the 1.x version of facets and it is regarding ajax, I'm going to close this issue. We are only supporting ajax with views in facets 3.x.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

Since this is in the 2.x version of facets and it is regarding ajax, I'm going to close this issue. We are only supporting ajax with views in facets 3.x.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

Since this is in the 2.x version of facets and it is regarding ajax, I'm going to close this issue. We are only supporting ajax with views in facets 3.x.

PS: It seems you (like many others – it's really easy to misinterpret) are confused by the "Issue tags" field. As the guidelines state, they aren't meant for free text tags related to the issue, but only for specific categorization purposes, usually by module maintainers.
So, if you aren't sure your current usage is correct, please just leave the field empty.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

Since this is in the 2.x version of facets and it is regarding ajax, I'm going to close this issue. We are only supporting ajax with views in facets 3.x.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

Since this is in the 1.x version of facets and it is regarding ajax, I'm going to close this issue. We are only supporting ajax with views in facets 3.x.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

You're right, we should update this, only in the 3.x version of the readme.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

I agree this is complex, however the validation we would need to check all options is not an option.
I am closing this one out.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

Since this is in the 2.x line, but about ajax, I'm not sure how to feel about this. I don't want to risk breaking anything in that part, but this should no longer work on 11.x.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

Since this is in the 1.x version of facets and it is regarding ajax, I'm going to close this issue. We are only supporting ajax with views in facets 3.x.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

Closing this issue, just like @timotej-pl I can't reproduce this.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

Yes, that seems logical to first do operations on the tree before transforming it into hierarchy.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

To me, this looks like an issue in drupal's installer process? Does this still happen? If it does we need to look at the dependencies of our own schema.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

Since this is in the 1.x version of facets and it is regarding ajax, I'm going to close this issue. We are only supporting ajax with views in facets 3.x.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

You are seeing this because it is just rendering the content coming back from the index. This should be resolved with the EntityTranslationProcessor.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

The other module is what should be used for the 1.x and 2.x lines of the module. For the 3.x version this should be done through an exposed views filter.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

Since this is in the 1.x version of facets and it is regarding ajax, I'm going to close this issue. We are only supporting ajax with views in facets 3.x.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

Because this is a static cache, it's impossible in my eyes for this to happen. I would like to have more information on this issue, but closing it as outdated for now, feel free te reopen with extra information if it still happens.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

I don't understand the actions we'd need to do to hep with this issue. Closing this

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

I think most problems with this should be resolved when we get #2414951 working. I think that means we can close this issue.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

I agree that we should close this issue. It's no longer relevant.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

@longwave what's a good way for testing this one?

Not longwave, but I can answer that as well. phpcs -e shows you the list of sniffs being executed, so they need to be compared before/after checking out this branch. They should be the same.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

As you say, there's no way to do this without also having to do entity queries, ruining performance. Closing this issue.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

Facets 3.x can be exposed filters based on views, so that should work for your usecase.

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

I don't know what realname is? Can you please provide more information?

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

Closing this as outdated, since hopefully most sites are now > 8.5.x

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

Thank @Todd, that should be it. Closing this

🇧🇪Belgium borisson_ Mechelen, 🇧🇪

Since this is in the 1.x version of facets, pre-stable, I'm going to close this issue.

Production build 0.71.5 2024