This came just down to being a stupid invert. !empty, rather than empty(). Wonder how long this has actually been this way. 😆
thanks for the insights, Adam.
thanks, all
chrisfromredfin → created an issue.
Reviewed & tested here, too. G2G
chrisfromredfin → made their first commit to this issue’s fork.
Please note that this is still only a "may" - we definitely want to profile before and after the patch to see how much time is actually saved. If we exclude copying core into the sandbox and back out, Composer is still going to re-install it for us in the sandbox. So is it faster to use Composer to re-install, or is it faster to copy it in so it doesn't have to?
Profile, profile, profile.
The work here is maybe woefully outdated and may need to be "ported" to 2.0.x and the Activator system. That said, there is still the possibility that a project_machine_name might not match the underlying module_machine_name.
For complex scenarios, we really should just lean on recipes, but a small improvement here would increase the reliability of Project Browser.
Updating IS.
chrisfromredfin → created an issue.
chrisfromredfin → changed the visibility of the branch 3487845-make-the-overall to hidden.
This will get people to their projects faster!
I figured out that the searchString is gone now in favor of $filters.search so I updated that and it worked. But then I figured it needed a test, and i couldn't make the test work (which is weird it really should...)
So, I'm moving the behavior change to a follow-up and merging with just the CSS fixes that are definitely more in-scope. The follow-up should fix it and have a test.
scissors.gif
chrisfromredfin → made their first commit to this issue’s fork.
nice little ux improvement here!
chrisfromredfin → made their first commit to this issue’s fork.
Thanks for the look, Sandip! Happy to have you here (especially if you have some Svelte chops!) :)
yeah boi
chrisfromredfin → made their first commit to this issue’s fork.
We made it!
@goodboy, AHA! OK, so what you're actually seeing then is a delay in when we update the information in Project Browser. Actually, our endpoint is running off a D10 version of Drupal.org where we migrate the data from the existing drupal.org to that one on a nightly schedule.
It may take a while for the canonical data to move from drupal.org over to the new site.
I'll ask @fjgarlin how long we should expect data to update, or if we're using `--update` with our migration.
Greg you’ve pretty much hit the nail on the head with regard to how it’s working.
The area of interest you’re describing is in Package Manager, which is now in Drupal core.
Automatic Updates and Project Browser both rely on Package Manager to handle the sandboxing and Composer operations.
I’m not sure if PM would accept a request for this. If I were in charge I would! I will bring Adam into this thread and see what he thinks, as he was largely responsible for the package manager code.
This looks really good from a code perspective, but at least one bug.
It seems like ones that should have the default/fallback logo are instead using the logo of the previous project that has one set. so in 2.0.x, Scheduler has a logo, but File Entity does not. Feeds does, but the three after Feeds do not, etc.
Once you get out of the top 100 you see a bunch that don't have logos (my favorite is going to page 5) on the drupalorg_jsonapi source.
IT'S ALL IN $filters NOW!!!!1
chrisfromredfin → made their first commit to this issue’s fork.
chrisfromredfin → created an issue.
I agree with this behavior change. We are trying to make each source a bit more independent, and this gets us there. We may want to, in the future, do a feature request to persist the pageSize per source but I am going to wait for someone to ask for it. It may not be a big enough deal.
chrisfromredfin → made their first commit to this issue’s fork.
someone filed this a long time ago and closed with CNR - glad we found it again and fixed it
chrisfromredfin → made their first commit to this issue’s fork.
Moving back to NW.
You can set max_selections to one with drush or some other way to edit your config:
drush cset -y project_browser.admin_settings max_selections 1
This completely removes the dancing dots, even when we need them, which is when max_selections is 1.
Alternatively, the spinner we use could be put up when max_selections = 1, and that would also be an OK alternative (perhaps even better, meaning consistent UI both ways).
big bang for the buck. this bothered me so much! thanks, everyone :)
tested, looks great. simple code and doesn't need a test.
much clean. very amaze. still references to gin in there directly, of course, but that's for another day... this gets us there.
chrisfromredfin → made their first commit to this issue’s fork.
Haha, sorry about that! I must've had too many issues open and dropped those tags on the wrong one! 😬
baller
chrisfromredfin → made their first commit to this issue’s fork.
ship it real good
I think with automated tests passing for a known reason, and the code changes being grokable, and me manual testing, I think this one looks good!
chrisfromredfin → made their first commit to this issue’s fork.
chrisfromredfin → created an issue.
chrisfromredfin → created an issue.
chrisfromredfin → created an issue.
chrisfromredfin → created an issue.
Re the "chaining" of wizards... we already do this in core - you can add multiple fields to a view in one go, and then you configure each field in the order you added them. I would propose the same for the post-apply modals.
I just want to consider if the solution is architectural - as in, should there be two different AI recipes, one for the chatbot, one for the other... I'm not 100% sure I'm understanding the use case, but I'm not sure that skipping the config is smart. I would honestly rather a user get confused and abandon applying the recipe rather than end up in a weird state. If we wanted to do an "accept defaults" button, that would be good, and we could maybe have recipes provide that as an option where it makes sense??
This is related to 📌 Cleanup of Project object contract/constructor Active
Leslie reminded me today that if Package Manager validation fails at the page-load, we default to showing "View Commands" instead of "Install" or "Select."
If we move the readiness check to the beginning of install, that does push off when the error is being reported to the user. I think that's fine, BUT, if we then cache the PM validation check in some way, we'd be able to switch back to the 'View Commands' fallback. Is there any intention to handle this? It's kind of a nice feature now that we may lose... ?
The other thing to note is that if you ask for it, then update, Project Browser tends to aggressively "cache" data that we've requested from the endpoint. This is not in regular cache, but in key-value storage (for... reasons). So PB has a UI where you can flush that data. Then you might start seeing the data in PB as long as you're seeing it on your project page.
Config > Development > Project Browser > Actions.
If people can browser test this on Drupal 11, with Firefox, Safari, and some Chrome-based browser, they can move this to RTBC.
https://drupal.slack.com/archives/C01UHB4QG12/p1738768229634219
Postponing for the same reason as ✨ Update local task titles to match Drupal CMS changes Active - waiting for clear direction from a UX team who is working on this problem.
Per https://drupal.slack.com/archives/C01UHB4QG12/p1738768229634219 - I am going to postpone this until we have a clear direction from people who have thought about this UX more intentionally and holistically. (@ckrina is spearheading that, but it's not ready yet)
moving back to NW for now
The more I think about it, the more I think that we need to provide a "local contrib" plugin, and not commingle custom, contrib, and core into one backend. They're just a bit too different to do that. If we _have_ to, then we could merge custom and contrib, but I still don't want to. Here's why:
- We already have a core plugin that works.
- A contrib ProjectBrowserSourcePlugin could be made that scans a given folder (perhaps this is configurable? defaulting to web/modules/contrib), and then fetches and caches metadata for those projects from Drupal.org. In this way we can provide the best fully-featured experience for the trial experience but it restricts the ability to download by the nature of it only showing ones locally available.
- A "custom" ProjectBrowserSourcePlugin could be made that operates much the same way, but uses data from the .info.yml, much like the core plugin does today.
I see this being a good UI, where each tab is Core | Contrib | Custom. (I would really want to get in ✨ Expose sources as local actions underneath a "Browse" local task Active )
ALSO, I want to point out that there's a facility to write these plugins in a separate contrib module right now, nothing is stopping anyone. I wonder if that's the best way to pioneer this for the Trial Experience benefit, tho the way I've architected it above, I would certainly consider it as an MR.
And, by no means is this the end of the discussion, I'm sure I'm thinking of a number of things I'm still not thinking of. :)
Moving to NW just to get an answer to my question about that other class. but it may be out of scope for this issue .
<borat>very nice</borat>
chrisfromredfin → made their first commit to this issue’s fork.
Thanks for the review. Manually testing showed this is good!
chrisfromredfin → created an issue.
I know in the Slack channel people were discussing alternatives of Drupal hosting along the lines of SimplyTest.ME, Tugboat, etc. The fundamental difference there being that what is missing is the contributing aspect - the full IDE-in-browser experience.
To that, I know that while we have concerns about things "not staying around" (as GitPod Classic didn't, for example), I may recommend Drupal Forge as something to at least have a discussion with/about. It DOES have the full-featured IDE, and we are close to these folks inside the Drupal Community.
What I think is missing is the concerted effort to spin up code from an MR, for example. But that might be an easier lift for folks already working inside the IDE-in-browser space. ?
I have no existing relationship or knowledge of them, except having seen a quick demo at DrupalCamp Asheville. But the conversations in Slack had me thinking.
chrisfromredfin → created an issue.
I still have some FU to file for this - the styling is still off in Claro (like 2px height differential) but that's been there forever and is out of scope for this issue. And this is an improvement to dark mode / Gin and will help Drupal CMS. So, here goes!
signed, sealed, delivered.
chrisfromredfin → made their first commit to this issue’s fork.
chrisfromredfin → made their first commit to this issue’s fork.
Whoa.
chrisfromredfin → made their first commit to this issue’s fork.
chrisfromredfin → created an issue.
[p.s. Reviewed this with Adam over Zoom, tested thoroughly manually. No changes with the exception of keeping the Clear/Recommended links present always, which was previously discussed.
Follow-up coming on whether or not that's the better UI, or if it can be improved in a different way. But it was taken for the pro of it being less/easier code.]
chrisfromredfin → made their first commit to this issue’s fork.
Thanks for the help! Great to see some new names in the queue :)
Confirmed working, even with manual testing (found a module that errored). Nice improvement!
Another great diffstat AND I really did a ton of manual testing. This is such a great unblocker for future source plugins.
chrisfromredfin → made their first commit to this issue’s fork.
chrisfromredfin → created an issue.
re-ran tests and they passed. will merge this soon, after the larger categories one.
chrisfromredfin → made their first commit to this issue’s fork.
ok, let's keep slicing away at these old mechanisms!
🔪
step in the right direction...
chrisfromredfin → made their first commit to this issue’s fork.
I rebased this to test and push, but now the NEW test is failing. NULL is not empty?
Clean up error messages, always a better UX.
chrisfromredfin → made their first commit to this issue’s fork.
thanks for the work, weekend warriors!
chrisfromredfin → made their first commit to this issue’s fork.
A note on that - the spinner I believe prior to this issue was in the center of the screen. If we can just restore it that way, that's fine for the scope of this issue (but it does seem to be a regression introduced by this issue).
That said, if it's easy to move the spinner into the bar, that's a better UI - since the indicator is nearer the context of the action that caused it. So if you can do that, all the better. :)
After the rebase it seems we lost the "sticky" feature of the bar once you start adding items into the batch.
Also, while applying the batch, the spinner is missing from the center of the main screen. We need SOME kind of indication that something is happening, or users will definitely be confused. Especially since their ability to interact with things has still disappeared.
manually tested and verified. "The light is green, the trap is clean!"
chrisfromredfin → made their first commit to this issue’s fork.
We currently have Nightwatch tests up and running again. The keyboard-based navigation support in Nightwatch is superior to what we can do in Mink / FunctionalJavaScript tests with PHPUnit.
I think this will probably eventually be Closed (won't fix) as we'll likely convert the Nightwatch tests to Playwright in 📌 Rewrite Nightwatch tests as Playwright Active .
Essentially now, the implied contract is that summary is required but value is not. This generally will work better for "local-system" types of source plugins (like current Recipes).