Closed (fixed)
Project:
Module Filter
Version:
4.x-dev
Component:
Code
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Reporter:
Created:
21 Jan 2023 at 16:30 UTC
Updated:
17 Jan 2024 at 09:09 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
jonathan1055 commentedLinked to the wrong issue.
Comment #3
jonathan1055 commentedSorry, having trouble typing. I have cut my finger so using the wrong hand (well that's my excuse). Also forgetting to 'preview' :-)
Comment #4
jonathan1055 commentedAdded test first. This should fail.
Comment #6
jonathan1055 commentedThe log has
cd ${SOURCE_DIR} && sudo -u www-data curl https://git.drupalcode.org/project/drupal/-/merge_requests/3273.diff | sudo -u www-data patch -p1 --verbosebut there is no actual confirmation that the patch was sucessful. Anyway, lets try with the modified filter.Comment #8
smustgrave commentedpostponing until #2895388: Modules uninstall filter does not filter by machine name is merged.
Comment #9
jonathan1055 commentedThe second patch I added here includes the small change in the first patch, and the patching failed, so I removed the first one.
Comment #10
jonathan1055 commentedThat's good in a way. We now have the drupalCI test failure matching the gitlab pipeline test failure, so we are at least testing the same thing and getting the same outcome.
The above failure is the new bit added to the uninstall test, to demonstrate the new functionality.
The DrupalCI tests were passing in Jan 2023 but failing in Nov 2023. Same D10.1, php 8.1.
Attached are screen shots of the downloaded html output. The filtered page is indeed not showing any modules at all.
Comment #11
jonathan1055 commentedThe previous image did not have the description opened. This one shows the 'red' text which should be found and filtered on.
Comment #12
jonathan1055 commentedTested this in the UI on 10.1 and 10.2 and it fails exactly like the javascript test. It also does not seem to filter on the module readable name either. But it does filter on the description (which is really weird). Using core modules for testing:
Readable name
Machine name
This used to work properly, so I don't know what has changed and whats gone wrong.
Comment #13
jonathan1055 commentedI have found and fixed the problems:
.module-machine-nameused to be on the td cell itself, but has now been moved into the span element..module-list__module-name. It remains as.module-namein Olivero [the commit msg should say Claro not Olivero]Tests pass. Needs review.
Comment #14
smustgrave commentedDon't seem to be able to filter by machine name. Did a fresh install and searched for block_content and got no results.
Comment #15
jonathan1055 commented"block_content" It works for me. Although I am having real problems with getting any chnages to .js to be recognised in a timely fashion. I do not have javascript aggregated, and have no caching, I clear the cache via Devel, via admin/config/development/performance and via drush, and the updated .js is not acted on. It makes development impossible. I even restart my local server. But then I wait 5 mins, do something else, and come back to module_filter and the updated code is active. Its driving me nuts!
Comment #16
jonathan1055 commentedoops, did not mean to change the status. I just refreshed the page, made my comment, after reading yours, but the stale form setting got saved.
Comment #17
jonathan1055 commentedI forgot to ask what theme(s) you are using to test? I checked this lastest change for Claro and Olivero in 10.1 and 10.2.
Actually there may be a safer, more consistent and future-proof way to identify the elements for text filtering. There is a class
table-filter-text-sourcewhich would seem to be designed specifically for this type of filtering. It is on the module name and machine name in both themes, but it is not on the description. But that will still simplyfy the coding.Comment #18
smustgrave commentedNeeds a rebase, also we can do it here or a follow up but think going to drop drupalCi file in favor of just gitlab.
Comment #19
jonathan1055 commentedRebased locally and pushed. Ready for review.
From your problem in #14 and my question in #17 - What theme are you using when you can't get the filter to work correctly?
Comment #21
smustgrave commentedLong time coming but welcomes addition!
Comment #22
smustgrave commentedI should of marked Fixed
Comment #23
jonathan1055 commentedUpdated IS to include a note about patching core in our test files.