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

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

flocondetoile created an issue. See original summary.

flocondetoile’s picture

A first attempt to demo the feature

flocondetoile’s picture

Status: Active » Needs review
flocondetoile’s picture

Forgot to change the label

borisson_’s picture

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

Status: Needs review » Needs work

Back to NW for tests + small change

flocondetoile’s picture

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

Will add too the corresponding schema in the next iteration.

flocondetoile’s picture

Status: Needs work » Needs review
FileSize
3.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

Status: Needs work » Needs review

Relaunch failing tests

flocondetoile’s picture

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

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

joachim’s picture

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

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!