🇺🇸United States @chrisfromredfin

Fixed via 793 - trust the tools!

chrisfromredfin → made their first commit to this issue’s fork.

I will wait on tests, but this has been discussed in https://drupal.slack.com/archives/C01UHB4QG12/p1747231711012519 with Adam & Fran.

+1 to this.

This is a very intentional design decision we made after a lot of community feedback and focus groups. We are trying to specifically keep people OFF of Drupal.org unless they are developers who really want to get that additional info.

We are surfacing the most important information in the modal that should allow people to make the decision they need in at least the 80% use case.

We're not opposed to adding this - but we (a) need to think around SSO/TFA issues that Benji brings up, and (b) I think we need to think through the UI / user flow for this. I would imagine a modal might make the most sense, "interrupting" the process if we're not within the flood limit.

Agree to keep the dev mode settings CLI-only or in settings.php. I think a 'dev mode' UI would be a poor choice. If it's a dev, it's a dev - and you can count on them to modify settings.php, (even an Ambitious Site Builder could do this, IMO) etc.

The real reason for the discrepancy is that we are actually doing a nightly migration to a Drupal 9 site which hosts the endpoint for the project browser module data. New modules may take a while to show up, or something may fail in the migration.

(In addition to other things, like our filters - we will not show modules that are not compatible with the version of Drupal that's running, and we only show ones by default that have security coverage, etc.)

I will see if Fran can take a look at the migration to check on this.

closed (dupe)
https://www.drupal.org/project/project_browser/issues/3453746 📌 Test Project Browser on Infomaniak Active

Closing (outdated) - DrupalPod may no longer exist! Happy to re-open if there's a good solution that comes up.

I've updated this a little bit, but tests are still failing. Some other things I've noticed that need to be fixed:

(1) if you use the "Clear" button it doesn't cause a reload, and it should.
(2) If you check some categories and hit apply, but then use "Clear filters" it doesn't clear the checkboxes from the component.

The experience needs to be aware of all the ways categories may be changed/reset/cleared, etc.

Unassigning in case someone else wants to pick this up.

I'm speaking a bit outside my comfort zone here, but I know that we have used a Decorator pattern in order to create the "Local modules" Source plugin.

I don't know much about decorators, so I am just coming to ask the question - whether or not approaching it from that paradigm solves the issues or if we would still need to have these methods be protected instead of private?

That aside, I think @penyaskito's comment is actually what the intention was here, to bring in /recipes & /core/recipes together - but we were waiting on recipes being more stable to do so, or have some discoverability from d.o... though maybe that should have been marked with a @todo.

Well, thank you Pam! Good to know there's a reliable one working pretty readily!

Once you are in and have the project browser, are you able to add new projects using the automated installation feature? That's an important piece to know.

(1) I think doing this may then have ramifications for not being able to download it later via composer, since it will have the project name wrong?

(2) I think the actual issue is with the endpoint/API - it seems to be repeating the characters / name over and over. That actual project name is 31 characters long.

https://www.drupal.org/project/augmentor_google_cloud_text_to_speech →

We should coordinate with d.o and know what their max length is for project node titles (255?) and just use that on ours (we probably already are? But I haven't looked at the schema).

I'll see if I can get @fjgarlin to check out this project on the API site.

I believe the logo is being set by the recipe backend to be the png we don't like. So while this introduces a fallback, will the fallback be used for recipes?

https://git.drupalcode.org/project/project_browser/-/blob/2.0.x/src/Plug...

Do we need to change it there?

Dupe of 📌 Automated Drupal 11 compatibility fixes for browsersync Needs review now.

chrisfromredfin → made their first commit to this issue’s fork.

Committed this, leaving issue open per instructions in case more comes along. ;)

Thanks for the work!

It gets bad when you start to glaze over when seeing the red. Green is best!

chrisfromredfin → made their first commit to this issue’s fork.

The issue here is line 24 of project_browser.module which is using Url::fromRoute to link to a route which now requires a parameter.

I think the right thing here is to instead iterate over all enabled sources, and generate a link to each one:

      $output .= '<dd>' . t('Users who have the <em>Administer modules</em> can browse modules from the <a href=":project_browser_admin">Browse projects page</a>.', [':project_browser_admin' => Url::fromRoute('project_browser.browse', [])->toString()]) . '</dd>';

So it says something like:

Users who have the Administer modules ^permission can browse projects from the Extend section:

• Contrib modules
• Recipes

Something like that.

Cool! We can leave that todo... maybe some day we'll be there, or someday we'll just remove it. ;)

chrisfromredfin → made their first commit to this issue’s fork.

I'm not sure this needs to be postponed on that, as I don't think it will actually change the CSS. there's still always a chance of a dropbutton having one action, so I think we can move forward on this.

chrisfromredfin → changed the visibility of the branch 3458840-apply-clear-buttons-categories to hidden.

All of this sounds good to me. Only thing that I take a quick note of is that we already have the "dependency popularity" problem with modules... Chaos Tools being one of the top-installed modules... that users almost never care about.

Our consideration for this was to maybe allow projects to opt into a checkbox like "library-only" and then hide those from PB's default filters (allow them to be shown if the user wants that).

Not sure that applies to recipes, as most base recipes ARE "useful unto themselves," so maybe it's different. Anyway, just a little something to consider/think about.

Thank you, libbna! Good progress. A couple things I'm noticing.

(1) The apply/clear bar should be "sticky" at the bottom of the container that opens. That is, it should always be visible and only the list of checkboxes should be scrollable. That can probably also be a little less tall. Maybe something like a height of whatever 4.5 rows is, with maybe a max-height of like 90vh or something?

(2) the "Clear" button should act as a submit button. So, while it clears the checkboxes, you still have to scroll back down and click Apply, which shouldn't actually be necessary. Hitting clear should wipe out all the category selections and cause a projects refresh.

chrisfromredfin → changed the visibility of the branch 3458840-consider-adding-apply to hidden.

chrisfromredfin → changed the visibility of the branch 3515746-change-language-around to hidden.

I think because the fix is in Svelte code, we need a FunctionalJavascriptTest to render the JS.

It seems to me like the answer is to *improve* the config form, then. The recipe config form for AI should prompt not just for an OpenAI key, but for a Provider + Key. No?

chrisfromredfin → created an issue.

This does fix the problem so I'm going to merge it now. But I think if we can rely on native Drupal api's like Drupal.url() we need to do that, so I'll open that as a follow-up.

In sveltejs/constants.js, we use "export const FULL_MODULE_PATH = `${BASE_URL}${drupalSettings.project_browser.module_path}`;" and FULL_MODULE_PATH is then used to get the path to the image.

BASE_URL uses "export const BASE_URL = `${window.location.protocol}//${window.location.host}${drupalSettings.path.baseUrl + drupalSettings.path.pathPrefix}`;"

...does drupalSettings.path.pathPrefix include a language prefix?

Or, does the core moduleHandler do it?
`'module_path' => $this->moduleHandler->getModule('project_browser')->getPath(),`

...I'm pretty sure it's probably drupalSettings.path.pathPrefix, which is meant for the "Drupal is installed in a subfolder" scenario; however, if it's a language prefix, that should NOT be included.

Does anyone know how core handles this kind of scenario, where it needs a path to the module without the language prefix?

This will need a test.

chrisfromredfin → changed the visibility of the branch drupal-3317653-3317653-rolled-for-10dot4 to hidden.

chrisfromredfin → changed the visibility of the branch drupal-3317653-3317653-rolled-for-10dot4 to active.

chrisfromredfin → changed the visibility of the branch drupal-3317653-3317653-rolled-for-10dot4 to hidden.

chrisfromredfin → changed the visibility of the branch 3317653-rolled-for-10dot4 to hidden.

chrisfromredfin → made their first commit to this issue’s fork.

Confirmed, already working in Gin 4.0.6 light and dark mode... !

@sarahlorenzen - is there a patch file or an open Merge Request for the work (hopefully an MR)?

Just confirming that Package Manager is not a strict _requirement_ for Project Browser; PB functions without it (though not in as wonderful a way). But it is a soft dependency.

chrisfromredfin → created an issue.

chrisfromredfin → created an issue.

OK, NOW it's Fixed.

chrisfromredfin → created an issue.

Thanks! I will open a follow-up to break out the menu toggle button, since this issue was scoped with two intents.

chrisfromredfin → made their first commit to this issue’s fork.

I've wanted this for so long; this is such a superior experience in such a subtle way.

NOTE:

• I'm using a generic "GTM-xxxxx" where x is a digit pattern when installing.
• My PB settings allow multiple installs (max_selections NULL)

Updated IS with steps to reproduce

Initial manual testing here seems to imply this is working well. Small pieces of feedback:

(1) the dropbutton with only one action in it looks janky. Not sure if we leave the carat, or shrink the space, or what. But something.

(2) having an issue with 'Drupal2.$dialog is not defined' or something similar. I have seen this in other MRs so I don't think it's related to this issue. But I've seen it enough that it should have a FU.

Release Notes: "BooleanFilter is now a boolean filter."

chrisfromredfin → made their first commit to this issue’s fork.

no notes

chrisfromredfin → made their first commit to this issue’s fork.

Tim ended up fixing this in a separate issue

I SWORE I already provided feedback on this, but it's not here - might have been one of those ones that I started in an open tab, then closed the tab.

There's a few more major things here, and it might be that this branches out into an issue that needs to be worked on before this one:

1. As Adam says, we need to basically allow a "description" more generically. That's the first piece. FilterBase should support a description field (which accepts markup?).

2. If a description is set, then we should set a ? icon, I think, next to the label (not next to the select). I would use core's questionmark-disc icon (although I'm not sure if we can always count on that being there, but might make sense to try anyway - even if it's not guaranteed).

3. As we're doing now, I think it's OK to then open that description markup in a popup/modal.

4. I'm not sure where we're actually getting the names/descriptions from, but I think we'd want to actually have this queried from Drupal.org; not sure if that means it needs its own endpoint? I'm happy to do that part later, and rely on something like the test fixture (for now), but I don't love that as a long-term solution. (Also, happy to cache that for like an hour or even longer if needed, to help reduce requests to d.o)

back in green, i hit the spleen, it's been to long I'm glad to be seen... 🎵

chrisfromredfin → made their first commit to this issue’s fork.

Manually tested in Claro, Gin (Light & Dark). Works great, even fixes an oddity on 2.0.x with Claro. Thank you!

chrisfromredfin → made their first commit to this issue’s fork.

tests are passing, PHPStan issue present in 2.0.x-HEAD

chrisfromredfin → made their first commit to this issue’s fork.

PHPStan is from HEAD, so not an issue from this MR.

chrisfromredfin → created an issue.

chrisfromredfin → created an issue.

chrisfromredfin → made their first commit to this issue’s fork.

I wonder if @lostcarpark could turn this part of the testing into a Nightwatch test? I'm not sure if that would actually help or not. Maybe its scrollIntoView works better, or it handles clicking things that aren't visible better...?

confirmed fixed with manual testing.

chrisfromredfin → made their first commit to this issue’s fork.

Thanks for the work! Good to see you in the queue, penyaskito!

chrisfromredfin → made their first commit to this issue’s fork.

All my suggestions are being implemented relative to the pager ticket in ✨ UI enhancement Active so no need to worry about it here.

Downgrading to stable-blocking because we want this done before we say "we're ready for prime time!" but - it won't have API implications. I think originally was beta when we were considering experimental-in-core as beta.

Postponing, pending results of UX research that is being spearheaded around the tabs & navigation. We need to know the IA before we can continue; although we're not discounting merging core + contrib into one plugin, but we already have a non-API-breaking pattern for that.

No API considerations, moving to stable. HOWEVER, leaving on the SHOULD HAVEs for -beta1.

With no API considerations for this (other than the fact that these keys exist on the Project object, so apply to all sources), pushing back to stable.

After discussing APIs with Adam on Zoom, I think this can safely move to stable blocking. I do consider it a shortcoming that we can't be tolerant, but after looking at all the APIs and where they _might_ change as a result of implementing this, we don't think there's any breakage--if anything, just possibly an addition, if at all.

So I'm re-triaging to stable blocking.

saving to get re-draw ¯\_(ツ)_/¯

Leslie and I just discussed and we think the current experience is acceptable, especially with regard to our target audience (who likely will not have the install functionality disabled anyway), so we'll mark this as Closed (WAD).

If there's an intense clamoring from the community in the future, we'll re-open or re-address.

It's not worth the brain damage of writing all the code and tests for it.

Are there any rules around changing any "JavaScript APIs" for beta for Drupal? This probably got marked beta as a "boy it would be nice" so I'm ok downing it. I'm just gunshy over "API changes" - if we only care about the PHP API, then I'm comfortable because I'm sure most if not all of this solution is on the Svelte side.

That'll do pig, that'll do.

chrisfromredfin → made their first commit to this issue’s fork.

love that this is the level of detail we're able to pay attention to now - a testament to our stability!

chrisfromredfin → made their first commit to this issue’s fork.

better than status quo. nothing we should really prescribe about when toolbar is active. Using Navigation (as CMS does) will make it a non-issue ;)

chrisfromredfin → made their first commit to this issue’s fork.

yessireebob

chrisfromredfin → made their first commit to this issue’s fork.

confirmed with manual testing that the new additions fix the issues with token when there's a dependency installed. turns out there's a pattern in core where we check dependencies, and then everything else goes to a validator, so we need to replicate that here.

chrisfromredfin → made their first commit to this issue’s fork.

cool, and getting cooler!

I may follow a design FU but it shouldn't block this.

I'm OK with no test since (a) there's already coverage for this kind of, and (b) I'm SURE Darren will let us know if we regress it ;)

chrisfromredfin → made their first commit to this issue’s fork.

manual testing confirms what we already know. need to consider replacing usage of state in tests with key-value, but that's for another day

chrisfromredfin → made their first commit to this issue’s fork.