Actually we can only render a facet based on Date with a html list. This is a little ugly if we want display this Date facet beside other facets, based on taxonomy term for example, which are rendered with the DropDown Widget

It would be nice if we could have an option on the DateBasicWidget to render the actual list of link as the Dropdown widget do. Let's me know if this make sens for you ? I can work on it and try to provide a patch.

Thanks

CommentFileSizeAuthor
#12 datebasicwidget_with_dropdown-2862674-9.patch3.55 KBflocondetoile
#9 datebasicwidget_with_dropdown-2862674-9.patch3.55 KBflocondetoile
#4 interdiff-2-4.txt588 bytesflocondetoile
#4 datebasicwidget_with_dropdown-2862674-4.patch2.06 KBflocondetoile
#2 datebasicwidget_with_dropdown-2862674-2.patch2.06 KBflocondetoile

Comments

flocondetoile created an issue. See original summary.

flocondetoile’s picture

flocondetoile
Primary language French
Location Lyon
commented
StatusFileSize
new datebasicwidget_with_dropdown-2862674-2.patch2.06 KB

A first attempt to demo the feature

flocondetoile’s picture

flocondetoile
Primary language French
Location Lyon
commented
Status: Active » Needs review
flocondetoile’s picture

flocondetoile
Primary language French
Location Lyon
commented
StatusFileSize
new datebasicwidget_with_dropdown-2862674-4.patch2.06 KB
new interdiff-2-4.txt588 bytes

Forgot to change the label

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
commented
Issue tags: +Needs tests

Overall this looks great, we need to write a javascript test for it though. Thanks!

+++ b/src/Plugin/facets/widget/DateBasicWidget.php
@@ -41,6 +41,8 @@ private function granularityOptions() {
+      'default_option_label' => 'Choose',

This should probably also run trough a t method?

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
commented
Status: Needs review » Needs work

Back to NW for tests + small change

flocondetoile’s picture

flocondetoile
Primary language French
Location Lyon
commented

Re #5.

In fact this default option label is an user input, so this string must be translated from the translate UI (admin/config/search/facets/FACET_NAME/edit/translate/LANGUAGE/add).

But I noticed that this widget do not have a schema so actually we can not translate the label as we can for the dropdown widget.

flocondetoile’s picture

flocondetoile
Primary language French
Location Lyon
commented

Will add too the corresponding schema in the next iteration.

flocondetoile’s picture

flocondetoile
Primary language French
Location Lyon
commented
Status: Needs work » Needs review
StatusFileSize
new datebasicwidget_with_dropdown-2862674-9.patch3.55 KB

Patch reroll against HEAD + config schema added for datebasic widget. Default Label option is now translatable.
Back to need review for launching existing test, but still missing corresponding JavascriptTest.
I am not yet "comfortable" with JavascriptTest and I will try soon.

Status: Needs review » Needs work

The last submitted patch, 9: datebasicwidget_with_dropdown-2862674-9.patch, failed testing.

flocondetoile’s picture

flocondetoile
Primary language French
Location Lyon
commented
Status: Needs work » Needs review

Relaunch failing tests

flocondetoile’s picture

flocondetoile
Primary language French
Location Lyon
commented
StatusFileSize
new datebasicwidget_with_dropdown-2862674-9.patch3.55 KB

Relaunch tests with same patch #9

Status: Needs review » Needs work

The last submitted patch, 12: datebasicwidget_with_dropdown-2862674-9.patch, failed testing.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
commented

So the current tests are green, but we need to add test-coverage for this as well.

joachim’s picture

joachim commented

I think this is the wrong way to fix this. This means we have to add options to the date widget for all the other widgets -- list, dropdown, checkboxes. In the future, when other data types need special handling, the problem gets worse.

I've proposed an alternative at #2896229: move date handling to a processor, so date facets can use existing widgets.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
commented
Status: Needs work » Closed (won't fix)

Closing this issue in favor of the other suggestion.

This approach would lead to a ton of duplication and make the module harder to maintain - we're actively trying to avoid that.

Thanks for your work on this issue though @flocondetoile!