lostcarpark → made their first commit to this issue’s fork.
Changed Clear Filters button to also clear the search text.
Added testClearSearch
to text both "X" on search box and "Clear Filters" clear the search text.
Verified tests pass.
The generic filter sounds good, but transferring the work I did on ✨ Make the overall design of the browse page more compact and dense Active in here.
lostcarpark → made their first commit to this issue’s fork.
Well, doh! Didn't notice this has been merged and tried to add test.
Will add in follow-on issue.
I'm eagerly awaiting this issue to be ready for testing. I managed to patch Olivero to create a subtheme back under D9, but the patch wouldn't work for me in D10.
As a stop-gap, I've been using Jean Valverde's Olivero Subtheme Project. It links to this issue, and warns that it will soon be obsolete, but it doesn't seem to be quite yet!
I fully expect to recreate my subtheme as a Starterkit theme when possible, but just now Jean's project seems to be the best option for Olivero derived themes.
The change in the MR seems overcomplicated, and I'm not sure it fixes the problem that the module won't install if there's a Markdown filter already existing.
Wouldn't a much simpler solution be to move config/install/filter.format.markdown.yml
to config/optional/filter.format.markdown.yml
?
This would mean that if there's a existing Markdown filter, it would be left as-is, and it would be up to the site builder to apply Markdown Easy to it. I feel this is reasonable as we don't know what might be in an existing filter, so we shouldn't change it. I think expecting a manual reconfiguration of the text filter is reasonable.
But there is. and has been at least since Jan 13, 2022.
Can you point to the specific support for GitLab Flavor Markdown? I don't see anything in either CommonMark's build in extensions, or in community extensions.
Is it in CommonMark or another Markdown library? @ultimike has said he doesn't want to get into supporting multiple libraries, since that is one of the things that makes the Markdown module difficult to use.
I think the change looks good.
I did a manual test and verified the Markdown Smörgåsbord flavor was available. When selected, I was able to use the footnote markdown. Switching back the GitLab flavor caused the footnotes to not render, as expected.
[ Note, it would appear that GitHub does now support footnotes. However, I think you stick to the GitHub markdown provided by Common Markdown. It might be worth opening an issue for that library, though. ]
I like the implementation of the new hook.
As the hook change is a breaking change, should you consider a major version change (to 2.0.0) when releasing this?
Moving to RTBC.
Sorry, will get reviewed today.
I thought this would need a Svelte code change, but when I pushed that change it caused a test to fail. Looking at the fail, I needed to move a container class back, which was when I realised the Svelte change wasn't needed and it could all be done with CSS.
I've moved the Search box inside the grey filter area. I think this connects them better visually, and allows the spacing to be tightened up:
I also checked it doesn't break in mobile view:
I think there would be scope for a more radical change, but after discussing in Slack, there wasn't an appetite for that, so I've kept it to simple tightening of the spacing.
The class gets added to the <a>
link here, and as @lrwebks says above, then gets passed to the template as #more_wrapper_class
.
The class is then added to the wrapper <div>
here.
It definitely doesn't seem right that the same class gets added to both. A workaround for sites using Smart Trim would be to override the template, and only use $more_settings['class']
to set the class of the <a>
link, and set a different class for the <div>
in the overridden template.
I do think we want to avoid adding too many additional paramaters, as providing the template allows the output to be modified in more flexible ways.
I'm wondering should we apply a class to the <a>
link at all, since it can be pretty easily be specified in CSS by .class-name a
, but as @anybody says, that could be a breaking change for sites relying on it.
Agree we should wait for input from @markie or @ultimike.
roderik → credited lostcarpark → .
@realityloop, they are on the second Wednesday of every month at 20:00 UTC, which I think converts to between 04:00 and 07:00 depending where in Australia you are, which I agree is not a great time for participating. They are deliberately async, so you can continue to contribute to the conversations for at least a day, and it's not uncommon for threads to still be active for several days.
I've made a first stab at tightening up spacing. The changes I've made are all through CSS, so none of the tests should be affected.
Here's a preview of the current changes:
From Leslie's comment above, moving the search into the grey area would make sense. This would slightly change the hierarchy of the form, so some tests would probably need some rework.
From Chris and rkoller's comments on Slack, we want to avoid changing the order of controls for now.
Currently the "Sort by" label is above the drop-down, which makes the line containing it excessively tall. I wonder would it make sense to move the label to the left of the drop-down, where there's plenty of space (as an aside, the Sort by label has a colon, which none of the other labels do)?
Fixed the ESLint issue. Everything is green now.
lostcarpark → made their first commit to this issue’s fork.
The new trait is a neat way of providing the trim functionality for both the formatter and the process plugin. However, it was missing a use statement in the classes to actually implement the trait. I renamed it to SmartTrimTrait, as SmartTrim was conflicting with the name of the SmartTrim plugin.
Should the input and output of the trim()
method be typed? I considered giving it a string
type, but I wasn't certain it would always be a string.
Does it need a test of the migration?
Also note currently PHPstan (next minor)
is failing. The failure is in the TruncateHTML
class, which hasn't been changed by this issue, so I think it's caused by an upstream change, so I think it should be looked at by a separate issue.
lostcarpark → made their first commit to this issue’s fork.
lostcarpark → created an issue.
I've scribbled some ideas on the current layout...
Here are my thoughts...
- There are several places where there is a big vertical gap that serves no purpose. While whitespace is important, I feel these could be tightened up.
- The Search box covers the width of the page. In most cases search text won't be more than a couple of words, so this doesn't need to be anywhere near as big. My thinking is a much reduced search box alligned to the right.
- I'm not sure why the grey filters box is separate from the search text. Surely the search is also a filter?
- The Security Advisory, Maintenance, and Development statuses are all on/off toggles. Could these be combined into a drop-down of checkboxes, with a heading like "Status filters"?
- If the above were implemented, could we have a single row with "Filter by category", "Status filters", and "Search".
- There are two lines below the filters. The first with "Clear filters", any selected filter lozenges, and "sort by". The second line has the result count and list/grid selectors. Both lines have a lot of empty space. Could all of this be fitted on a single line?
- Finally, there is a line above the top row of results, that is touching the tops of the boxes. The two lines merge and look weird. If we have a line there should be a gap between it and the results. But I don't think there's any need for a line, so we should get rid of it.
Nightwatch tests now working for all current versions.
Jonathan had enabled "previous major", so I tried leaving that in, but one of the FunctionalJavascript tests was failing, and fixing that would be outside the scope of this change, so I turned it off.
I've had some success at last.
I can run Nightwatch locally, but it doesn't seem to be running with the same settings as in GitLabCI, as the tests are failing on things that pass on GitLabCI.
However, I have been able to get enough information from the local runs to fix the tests so they run in GitLabCI.
The main issue seems to be the browser.keys()
call, which is deprecated, and I was using to send keypresses. I have replaced with browser.perform()
, and send the appropriate key in the callback function as a user action.
The next step is to rebase for the recent changes, and add back the tests that were disabled for this issue.
I think trying to implement the simplest possible solution makes sense, otherwise the conversation is likely to drag on another 2½ years...
A modal popup activated by a "?" icon outside the category drop-down would have several big advantages:
- It avoids adding complication to the Category drop-down JavaScript, which is already quite involved
- It keeps the user on the page, rather than diverting them to an external page, avoiding load time and having to use the back button to return
- It should be quick to implement
Perhaps a more complex solution could be considered later, but at least this approach should get a basic solution merged quickly.
Thanks Chris, I'll keep working on the Nightwatch tests.
mradcliffe → credited lostcarpark → .
mradcliffe → credited lostcarpark → .
mradcliffe → credited lostcarpark → .
mradcliffe → credited lostcarpark → .
mradcliffe → credited lostcarpark → .
mradcliffe → credited lostcarpark → .
Does PB not support D10 any more?
I've rebased this issue against the latest 2.x changes.
There were a number of changes to the UI that affected the Nightwatch tests, including changes to the labels and values in the category drop-down, as well as changing the project browser route to include the source plugin.
As the new route can take the browser directly to the project-browser-test-mock source, I've taken out the config step that was disabling other sources.
I'm still running the older Nightwatch version on my local, so I'll do some more on this as soon as I figure out how to upgrade it.
lostcarpark → made their first commit to this issue’s fork.
Verified the ESLint and CSpell issues are fixed.
There is an issue open for the Nightwatch test: 🐛 Nightwatch "next major" failing Active
I'm suggesting we merge this issue and leave the Nightwatch tests for the above one.
I think this should be ready to move to RTBC.
I'll feed back the side note, however checking multiple branches sounds problematic at best, as if two non-default branches have different logo.png files, how do you decide which takes precedence? Also, could trigger a lot of extra processing on a module with a lot of branches.
Hi,
The logo has been added to the 5.0 branch, but the default branch is still 4.0.
The logo needs to be in the default branch to be picked up by the project page and Project Browser.
Would it be possible to also add the logo to the 4.0 branch?
lostcarpark → created an issue.
lostcarpark → created an issue.
lostcarpark → created an issue.
lostcarpark → created an issue.
lostcarpark → created an issue.
lostcarpark → created an issue.
lostcarpark → created an issue. See original summary → .
lostcarpark → created an issue. See original summary → .
lostcarpark → created an issue. See original summary → .
lostcarpark → created an issue.
lostcarpark → created an issue.
lostcarpark → created an issue. See original summary → .
lostcarpark → created an issue.
lostcarpark → created an issue.
lostcarpark → created an issue.
lostcarpark → created an issue.
lostcarpark → created an issue.
lostcarpark → created an issue.
lostcarpark → created an issue.
lostcarpark → created an issue.
I've done some optimisation of the SVG file, and got it down to 2.5KB.
lostcarpark → created an issue.
drumm → credited lostcarpark → .
I have uploaded PNG and SVG versions of the logo, both sized to 512x512 pixels.
Here's a preview of the PNG. I had to give the SVG a .TXT extension, so I don't think I can embed here. However they should both look the same unless you zoom in.
In terms of file size, the PNG is 9KB, while the SVG is 5KB. Both are under the Project Browser requirement of 10KB.
I would favour using the SVG since it's smaller, and we already use a SVG for the module default image.
lostcarpark → created an issue.
lostcarpark → created an issue.
lostcarpark → created an issue.
lostcarpark → created an issue. See original summary → .
lostcarpark → created an issue.