[VUFIND-1614] Modernize PubDateVisAjax recommendation module #4275
Conversation
demiankatz
added
accessibility
architecture
There was a problem hiding this comment.
I think this interface is a nice improvement over the existing tool. When it's activated, the user can be very specific about the date range they want to see via the sliders or the text entry boxes.
In the existing tool, the user can only drag to select, it's not clear how to do that, and the selection is very imprecise. Also, if the user wants to change their selection, they can't adjust on one end only; they need to redraw the entire selection.
In any case, the new tool improves functionality! But I have a couple of critiques and questions.
NB: the universe of publication dates in the test data goes from 1783 through 2016.
Test:
Dev:
True/False setting is not functional
In searches.ini, this is how Demian suggested I set up this tool:
default_top_recommend[] = "PubDateVisAjax:true:publishDate"
With the option set to 'true,' when the user selects a date range the timeline zooms in and only shows the specified dates plus a buffer of five years outside those boundaries:
Test
Dev
If the option is changed to 'false' on test, the zooming in doesn't happen. The user selects a date range, and the graphic shows the shaded area for the date range of interest in the context of the full date range for the entire universe of materials.
This 'false' setting is functional in the dev branch, but not in the new tool on the test branch.
Test (display is the same with setting at true or false)
Dev
Visibility of text entry boxes and Set and Clear buttons
I don't like that the controls (text entry boxes and set and clear buttons) are only visible after you begin interacting with the slider, and also disappear after you clear your selections.
I understand that it saves vertical space to hide those controls when the user is not interested in interacting with the slider, so I'm not sure what the best solution is here. Could there be an option in searches.ini that would allow the library to specify whether the controls are always visible vs. only visible during use?
Or maybe the default should be that the actual years covering the collection are selected with the shading and data entry boxes, and all of the parts of the tool are visible at all times? The current default seems colorless when you compare it with the tool when it's "in action."
Year-specific publications
It's a nice touch that the user can mouse over a specific bar in the graph to see how many items were published in that particular year. However, it's frustrating that the bar and/or pop-up are not clickable to limit the results to ONLY the nine items published in 1992.
General aesthetics
-
I prefer the shorter height of the date range display on dev.
-
I would prefer a more attractive color for the shading, or the option to specify the color. The brown is unappealing to my eye.
-
I like the faint vertical lines that appear at 50 year intervals on the dev version. The blank field behind the timeline on test is a little too vast.
-
Would it be possible to emulate dev in displaying the year range in "round" numbers? It would be nice to always have these more standardized years displayed on the graphic. You can see the examples in the screenshots above; here's a textual description of the differences between test and dev:
-
The labels on dev before limits are applied show 1800, 1850, 1900, 1950, and 2000. The labels on test show 1778, 1803, 1828, 1853, .... 1953, 1978, 2003.
-
After limits are applied, dev still displays the graph with labels at every five years (1970 1975 1980 ... 2010 2015 2020) while test now uses a different set of non-round numbers (1970 1976 1982 1988 1994 2000 2006 2012 2018)
-
|
Thanks @sturkel89, those are some good points! True/False setting is not functional Visibility of text entry boxes and Set and Clear buttons Always show controls Hide controls initially Year-specific publications General aesthetics
|
There was a problem hiding this comment.
Thanks, @ThoWagen ! I think this is looking better and better. It is almost perfect!
I like the new more vertically compact layout, the "round" years, and the presence of tick marks. I also see that zooming works well.
Here are a couple of screenshots:
Bootstrap5:
Sandal5:
Tick marks
My critique of the tick marks is that I'd prefer if they lined up with the years instead of being between years. I am inclined to create my custom date range by dragging from tick mark to tick mark, but then I end up with "odd" years selected. This is the selection I made that resulted in the outcome above:
Could the tick marks be aligned with the year labels, as they are on dev?
Colors
I like the blue much better than the brown! I think the color looks great in bootstrap5.
I think the color seems a little bit intense in sandal5 since there are other strong colors there in the header -- however, it's not terrible. It might look better if it repeated one of the lighter colors that's already in use on the page -- from the breadcrumbs bar, or background of the Reset Filters button, or the background of the sidebar.
Settings confusion
You showed this example of the settings line with controls always displayed:
default_top_recommend[] = PubDateVisAjax:true:false:publish_year
I have been using this setting, based on Demian's suggestion to me when I first checked the PR:
default_top_recommend[] = PubDateVisAjax:true:publishDate
I'm finding that if I change the last bit to publish_year, I don't see the slider at all. Only publishDate works for me. Is this expected?
(I see that the true:true setting hides the text entry and set/clear buttons initially, and the true:false setting shows them all the time, so that improvement works!)
|
The grid is now aligned with the labels and I adjusted the color in sandal5. What do you think @sturkel89? And we use a different facet name locally so I accidentally used that in the example. With the standard VuFind index you should use |
There was a problem hiding this comment.
Thanks for making these adjustments, @ThoWagen! The tick marks now align with the "round" years, and it's easy to use the sliders to select those round years. The gray color in Sandal5 looks better to me than the more intense blue.
You've done so much, so I hesitate to ask for any other changes. I'll report my TINY new thought/question: could the bars in the graph be solid instead of outlined? I think the feature looks a bit busy the way it is, and would be easier on the eyes if the bars were filled in with the same solid color as the outline.
I'll submit this as a comment and will be happy to change to 'approve' after hearing back from you. Thanks!
|
@sturkel89 I agree that solid bars looks better. I only changed it for sandal5 for now, since I was not sure if you only meant that. And please feel always free to always suggest improvements! I'm happy to invest the work if there is room for improvement :) |
There was a problem hiding this comment.
That looks great!!
I would suggest making the same change to bootstrap5 as well. It would surely be an improvement.
I'm not sure what you were planning to do, but in my opinion the default setting should be:
default_top_recommend[] = PubDateVisAjax:true:false:publishDate
I think this feature will be completely ready to merge once the bars are filled in in the second theme. Thanks so much!
|
@sturkel89 Now the bars are also filled in the bootstrap5 theme. The default for zooming has already been "false" and I don't think we should change that now. But the controls are always visible by default. |
There was a problem hiding this comment.
Thanks, @ThoWagen, for all the improvements, and @sturkel89 for the thorough testing and feedback!
See below for a few minor suggestions from a review of the code.
There was a problem hiding this comment.
Thanks, @ThoWagen -- nearly done, but I think there might be just one case of using the wrong kind of escaping in the wrong context.
| <?=$this->results->getUrlQuery()->asHiddenFields(['page' => '/./', 'filter' => "/^{$facetField}:.*/"])?> | ||
| <input type="hidden" name="daterange[]" value="<?=$this->escapeHtmlAttr($facetField)?>"> | ||
| <form class="datevis-form facet-range-form hidden" name="datevis<?=$facetFieldEsc?>xForm" id="datevis<?=$facetFieldEsc?>xForm" data-name="<?=$facetFieldEsc?>" data-filter-field="daterange[]"> | ||
| <?=$this->results->getUrlQuery()->asHiddenFields(['page' => '/./', 'filter' => "/^{$facetFieldEsc}:.*/"])?> |
There was a problem hiding this comment.
I don't think we want to use $facetFieldEsc here, since this is a regex rather than HTML.
There was a problem hiding this comment.
Sorry, don't know how I overlooked that. Fixed.
There was a problem hiding this comment.
Actually, should we use preg_quote since that is a regex?
There was a problem hiding this comment.
Yes, I think preg_quote would be appropriate in this context. Thanks!
There was a problem hiding this comment.
Alright, I added it
In this PR a combination of charts.js and the date range slider from #4238 is used to replace PubDateVisAjax solution using the jQuery Flot library. The aim was also to increase accessibility and general user experience by using a slider and input fields instead of manipulating the chart with the mouse.