- πΊπΈUnited States smustgrave
This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request β as a guide.
Tagging. for an issue summary update following the standard template please.
This feature request will require test coverage as well
Thanks.
- πΊπΈUnited States pcate
@smustgrave I've updated the issue summary to follow the standard template.
- πΊπΈUnited States pcate
I've made an initial conversion of @dieuwe #10 patch to a merge request.
- πΊπΈUnited States pcate
It does appear the table style config schema needs to css class option added. The test I added was failing without it. Updated issue summary to reflect this.
- Status changed to Needs review
4 months ago 2:35pm 12 July 2024 - πΊπΈUnited States pcate
Setting to "Needs Review". I added a functionality test to confirm functionality is working, and I also tested manually.
- π©πͺGermany Anybody Porta Westfalica
GREAT work @PCate! RTBC! Removing the solved tags!
- Status changed to RTBC
4 months ago 3:00pm 29 July 2024 - πΊπΈUnited States smustgrave
Nice!
So this is one of those cases test-only feature doesn't work as it reverts the schema change. So applied locally and reverted the changes but kept the schema, so the test will run
Behat\Mink\Exception\ElementNotFoundException: Element matching xpath "//table[contains(concat(" ", @class, " "), " test-css-table-class1 test-css-table-class2 ")]" not found.
Which shows the test itself.
Believe this one is good. Only thing not 100% sure is if it will need a CR.
- πΊπΈUnited States dalemoore
Just testedβworking for me! This just made my day for reals.
Screenshots below.
- Status changed to Needs work
4 months ago 3:40am 8 August 2024 - π³πΏNew Zealand quietone
The issue summary clearly explains the problem and the change.
This is changing the UI, adding tag. The latest before and after screenshots should be available from the issue summary to help reviewers and committers. I updated the IS to state that they are in #26. Testing was done in #25 and manual testing in #26.
I read the MR and have commented on the user interface text.
- Status changed to Needs review
4 months ago 7:06am 8 August 2024 - π©πͺGermany Anybody Porta Westfalica
Thanks @quietone - you're right! Just did that, think you suggestion is good. I also added "Table" to the title to be even clearer about the context. Please review.
- Status changed to RTBC
3 months ago 12:15am 12 August 2024 - π¬π§United Kingdom longwave UK
Does this need an update hook to add the empty class to any existing views that use the table style?
- π¬π§United Kingdom catch
#30 is correct, we need a post update for that, otherwise there will be mis-matches when someone next resaves the views UI or a different update runs, marking needs work.
- Status changed to Needs work
29 days ago 7:41pm 23 October 2024 - πΊπΈUnited States pcate
Is there any documentation on how to write config update hooks for views? My understanding is that it is different than regular update hooks?
- πΊπΈUnited States pcate
Using existing views update hooks as examples I tried adding an update hook for the table CSS class schema change. I also added the update hook task to the list of remaining tasks.
- πΊπΈUnited States smustgrave
So with addition of upgrade path will need a test for the that hook.