Skip to content

Commit

Permalink
HTML Reporter: Fix bugs and improve usability of module filter menu
Browse files Browse the repository at this point in the history 
== Background ==

As part of fixing #1664, I
found that the minimal patch of simply appending the module HTML
on focus, no longer worked correctly in certain edge cases due to
various refactors that took place since.

This commit is the prep work to make the code simpler again, which
then makes fixing #1664 really simple. I also fixed numerous bugs
I found along the way, and improved the usability in a couple of
ways.

== Fix bugs ==

Most of these are specific to a narrow viewport, such as the iframe
on the https://qunitjs.com homepage!

* Dropdown menu was forced to 400px min-width, causing overflow
  on narrow viewports.

  Fixed for modern browsers by limiting min-width to 80vw.

* The dropdown menu was positioned too high in narrow viewports. The
  position was based on `top: 50%; margin-top: 0.8em;`, which
  approximated "1 line-height from the top".
  This worked in most viewports, where the dropdown label and input
  render next to each other. However, in narrow viewports, they wrap
  are render below each other, and "1 line from the top" is then
  overlapping half-way through the input field.

  Fixed by positioning the menu at `top: 100%` instead, which is
  always below the input field.

* The dropdown "arrow" icon was dislodged from the input field in
  narrow viewports due to being positioned absolutely on the right
  edge of the parent `<form>` and `<label>`. These are inline-block,
  which automatically follows the needed width, except when children
  wrap over multiple lines, then it reports its width as 100% even
  if there isn't anything in that space.

  Fixed by giving the parent element `text-align: right;` so that
  the dropdown menu and input field start from the right even in
  narrow viewports. On wider viewports this was already accomodated
  by `float: right;` for the then-single-line element.

* Dropdown items had `text-overflow: ellipsis;` but this did not
  work in practice because it was applied to the `<li>` element
  instead of the `<label>` where the text resides. The second reason
  is that the dropdown menu did not have a maximum width, which meant
  longer module names resulted in the menu getting wider instead of
  the options being trimmed with ellipsis. This is good and desirable
  when there is sufficient space.

  Fixed by seting a liberal max-width, and moving overflow to the
  `<label>` element, which the ellipsis will appear as-needed.

* There was an uncoloured padding between the line that divided
  the dropdown actions from the dropdown list, and the first selected
  item's background color.

  Fixed by removing padding from `#qunit-modulefilter-dropdown-list`.

== Usability improvements ==

* When searching for a module, selecting it, and then searching
  for something else; previously the only indication of what you
  selected so far was the "placeholder" of the input field.
  Apart from being cropped and thus not showing most of the info,
  this was by definition not visible when actually using the filter
  to search. It was only shown when not using the filter.

  Get rid of this placeholder, and instead hoist the selected modules
  to the top of the results, shown always, even if they don't match.

  To ensure a retention of vision and ofer an easy way to undo
  accidental clicks, we only hoist options when re-rendering the
  dropdown (e.g. after deciding to refine or change your search).
  When the results are idle, clicks simply toggle the options
  in their current position.

* Remove display toggling of the Reset/Apply buttons. Let these
  always be visible, with "Apply" always available, and the "Reset"
  button disabled when the selection is identical to the last-used
  (same as before).

* Replace the "All modules" checkbox with a "Select none" button,
  and for the common case where this is functionally identical
  to the "Reset" button, hide it to reduce clutter.

* Add tooltips to buttons to clarify their meaning.

* Add proper button styles, including high-contrast border
  and hover/active/disabled styles.

* For most viewports (wider than 350px) remove the reserved space for
  dropdown action buttons, and instead float them to the side and
  overlap the right side of the first dropdown result.

== Remove ==

Unused code:

```
 #qunit-modulefilter-dropdown a {
   color: inherit;
   text-decoration: none;
 }
```

Originally added in d14107a for the "All modules" action, which
was an anchor link at the time. This hasn't been the case of a while.
  • Loading branch information
@Krinkle
Krinkle committed Apr 17, 2022
1 parent 5479c77 commit 06c3951
Show file tree
Hide file tree
Showing 6 changed files with 242 additions and 135 deletions.
3 changes: 3 additions & 0 deletions .editorconfig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,6 @@ insert_final_newline = true
trim_trailing_whitespace = false
[test/integration/nyc.js]
trim_trailing_whitespace = false

[*.css]
indent_style = tab
14 changes: 13 additions & 1 deletion src/globals.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,12 @@ export const localSessionStorage = (function () {
// Support: IE 9-10, Safari 7, PhantomJS
export const StringMap = typeof g.Map === 'function'
? g.Map
: function StringMap () {
: function StringMap (input) {
let store = Object.create(null);
let hasOwn = Object.prototype.hasOwnProperty;
this.has = function (strKey) {
return hasOwn.call(store, strKey);
};
this.get = function (strKey) {
return store[strKey];
};
Expand All @@ -87,9 +90,18 @@ export const StringMap = typeof g.Map === 'function'
callback(store[strKey], strKey);
}
};
this.keys = function () {
return Object.keys(store);
};
this.clear = function () {
store = Object.create(null);
this.size = 0;
};
this.size = 0;

if (input) {
input.forEach((val, strKey) => {
this.set(strKey, val);
});
}
};
218 changes: 129 additions & 89 deletions src/html-reporter/html.js
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import QUnit from '../core';
import { extend, errorString } from '../core/utilities';
import { window, document, navigator, console } from '../globals';
import { window, document, navigator, console, StringMap } from '../globals';
import './urlparams';
import fuzzysort from 'fuzzysort';

Expand Down Expand Up @@ -50,7 +50,7 @@ export function escapeText (s) {
moduleId: undefined,
testId: undefined
});
const moduleSearchCache = [];
let dropdownData = null;

function trim (string) {
if (typeof string.trim === 'function') {
Expand Down Expand Up @@ -278,16 +278,9 @@ export function escapeText (s) {
function applyUrlParams () {
const filter = id('qunit-filter-input').value;

const selectedModules = [];
for (let i = 0; i < moduleSearchCache.length; i++) {
if (moduleSearchCache[i].checkbox.checked) {
selectedModules.push(moduleSearchCache[i].checkbox.value);
}
}

window.location = setUrl({
filter: (filter === '') ? undefined : filter,
moduleId: (selectedModules.length === 0) ? undefined : selectedModules,
moduleId: [...dropdownData.selectedMap.keys()],

// Remove module and testId filter
module: undefined,
Expand Down Expand Up @@ -342,32 +335,68 @@ export function escapeText (s) {
return filter;
}

function moduleListHtml () {
function createModuleListItem (moduleId, name, checked) {
const item = document.createElement('li');
item.innerHTML = '<label class="clickable' + (checked ? ' checked' : '') +
'"><input type="checkbox" ' + 'value="' + escapeText(moduleId) + '"' +
(checked ? ' checked="checked"' : '') + ' />' +
escapeText(name) + '</label>';
return item;
}

/**
* @param {Array} Results from fuzzysort
* @return {DocumentFragment}
*/
function moduleListHtml (results) {
const fragment = document.createDocumentFragment();

for (let i = 0; i < config.modules.length; i++) {
const mod = config.modules[i];
if (mod.name !== '') {
const checked = config.moduleId.indexOf(mod.moduleId) > -1;
const item = document.createElement('li');
item.innerHTML = "<label class='clickable" + (checked ? ' checked' : '') +
"'><input type='checkbox' " + "value='" + mod.moduleId + "'" +
(checked ? " checked='checked'" : '') + ' />' +
escapeText(mod.name) + '</label>';

moduleSearchCache.push({
name: mod.name,
item: item,
checkbox: item.firstChild.firstChild
});
fragment.appendChild(item);
// Hoist the already selected items, and show them always
// even if not matched by the current search.
dropdownData.selectedMap.forEach((name, moduleId) => {
fragment.appendChild(createModuleListItem(moduleId, name, true));
});

for (let i = 0; i < results.length; i++) {
const mod = results[i].obj;
if (!dropdownData.selectedMap.has(mod.moduleId)) {
fragment.appendChild(createModuleListItem(mod.moduleId, mod.name, false));
}
}
return fragment;
}

function toolbarModuleFilter () {
let dirty = false;
let initialSelected = null;
dropdownData = {
options: [],
selectedMap: new StringMap(),
isDirty: function () {
return [...dropdownData.selectedMap.keys()].sort().join(',') !==
[...initialSelected.keys()].sort().join(',');
}
};
for (let i = 0; i < config.modules.length; i++) {
const mod = config.modules[i];
if (mod.name === '') {
// Ignore the implicit and unnamed "global" test module.
continue;
}
dropdownData.options.push({
moduleId: mod.moduleId,
name: mod.name
});

// The module dropdown is seeded with the runtime configuration of the last run.
//
// We keep our own copy, because:
// 1. This naturaly filters out unknown moduleIds.
// 2. Gives us a place to manage and remember unsubmitted checkbox changes.
if (config.moduleId.indexOf(mod.moduleId) !== -1) {
dropdownData.selectedMap.set(mod.moduleId, mod.name);
}
}
initialSelected = new StringMap(dropdownData.selectedMap);

const moduleSearch = document.createElement('input');
moduleSearch.id = 'qunit-modulefilter-search';
Expand All @@ -378,60 +407,66 @@ export function escapeText (s) {
addEvent(moduleSearch, 'click', searchFocus);

const label = document.createElement('label');
label.id = 'qunit-modulefilter-search-container';
label.innerHTML = 'Module: ';
label.appendChild(moduleSearch);
label.htmlFor = 'qunit-modulefilter-search';
label.textContent = 'Module:';
const searchContainer = document.createElement('span');
searchContainer.id = 'qunit-modulefilter-search-container';
searchContainer.appendChild(moduleSearch);

const applyButton = document.createElement('button');
applyButton.textContent = 'Apply';
applyButton.style.display = 'none';
applyButton.title = 'Re-run the selected test modules';
addEvent(applyButton, 'click', applyUrlParams);

const resetButton = document.createElement('button');
resetButton.textContent = 'Reset';
resetButton.type = 'reset';
resetButton.style.display = 'none';

const allCheckbox = document.createElement('input');
allCheckbox.type = 'checkbox';
allCheckbox.checked = config.moduleId.length === 0;

const allModulesLabel = document.createElement('label');
allModulesLabel.className = 'clickable';
if (config.moduleId.length) {
allModulesLabel.className = 'checked';
}
allModulesLabel.appendChild(allCheckbox);
allModulesLabel.appendChild(document.createTextNode('All modules'));
resetButton.title = 'Restore the previous module selection';

const clearButton = document.createElement('button');
clearButton.textContent = 'Select none';
clearButton.type = 'button';
clearButton.title = 'Clear the current module selection';
addEvent(clearButton, 'click', function () {
dropdownData.selectedMap.clear();
selectionChange();
searchInput();
});

const actions = document.createElement('span');
actions.id = 'qunit-modulefilter-actions';
actions.appendChild(applyButton);
actions.appendChild(resetButton);
actions.appendChild(allModulesLabel);
let commit = actions.firstChild;
let reset = commit.nextSibling;
addEvent(commit, 'click', applyUrlParams);
if (initialSelected.size) {
// Only show clear button if functionally different from reset
actions.appendChild(clearButton);
}

const dropDownList = document.createElement('ul');
dropDownList.id = 'qunit-modulefilter-dropdown-list';
dropDownList.appendChild(moduleListHtml());
dropDownList.appendChild(filterModules(moduleSearch.value));

const dropDown = document.createElement('div');
dropDown.id = 'qunit-modulefilter-dropdown';
dropDown.style.display = 'none';
dropDown.appendChild(actions);
dropDown.appendChild(dropDownList);
addEvent(dropDown, 'change', selectionChange);
searchContainer.appendChild(dropDown);
// Set initial moduleSearch.placeholder and clearButton/resetButton.
selectionChange();

const moduleFilter = document.createElement('form');
moduleFilter.id = 'qunit-modulefilter';
moduleFilter.appendChild(label);
moduleFilter.appendChild(dropDown);
moduleFilter.appendChild(document.createTextNode(' '));
moduleFilter.appendChild(searchContainer);
addEvent(moduleFilter, 'submit', interceptNavigation);
addEvent(moduleFilter, 'reset', function () {
// Let the reset happen, then update styles
window.setTimeout(selectionChange);
dropdownData.selectedMap = new StringMap(initialSelected);
// Set moduleSearch.placeholder and reflect non-dirty state
selectionChange();
searchInput();
});

// Enables show/hide for the dropdown
Expand All @@ -441,10 +476,11 @@ export function escapeText (s) {
}

dropDown.style.display = 'block';

// Hide on Escape keydown or on click outside the container
addEvent(document, 'click', hideHandler);
addEvent(document, 'keydown', hideHandler);

// Hide on Escape keydown or outside-container click
function hideHandler (e) {
const inContainer = moduleFilter.contains(e.target);

Expand All @@ -461,19 +497,29 @@ export function escapeText (s) {
}
}

/**
* @param {string} searchText
* @return {DocumentFragment}
*/
function filterModules (searchText) {
let items;
let results;
if (searchText === '') {
items = moduleSearchCache.map(obj => obj.item);
// Improve on-boarding experience by having an immediate display of
// module names, indicating how the interface works. This also makes
// for a quicker interaction in the common case of small projects.
// Don't mandate typing just to get the menu.
results = dropdownData.options.map(obj => {
// Fake empty results. https://github.com/farzher/fuzzysort/issues/41
return { obj: obj };
});
} else {
items = fuzzysort.go(searchText, moduleSearchCache, { key: 'name', allowTypo: true })
.map(result => result.obj.item);
}
const fragment = document.createDocumentFragment();
for (let i = 0; i < items.length; i++) {
fragment.appendChild(items[i]);
results = fuzzysort.go(searchText, dropdownData.options, {
key: 'name',
allowTypo: true
});
}
return fragment;

return moduleListHtml(results);
}

// Processes module search box input
Expand All @@ -490,37 +536,31 @@ export function escapeText (s) {
});
}

// Processes selection changes
// Processes checkbox change, or a generic render (initial render, or after reset event)
// Avoid any dropdown rendering here as this is used by toolbarModuleFilter()
// during the initial render, which should not delay test execution.
function selectionChange (evt) {
const checkbox = (evt && evt.target) || allCheckbox;
const selectedNames = [];

toggleClass(checkbox.parentNode, 'checked', checkbox.checked);
const checkbox = (evt && evt.target) || null;

dirty = false;
if (checkbox.checked && checkbox !== allCheckbox) {
allCheckbox.checked = false;
removeClass(allCheckbox.parentNode, 'checked');
}
for (let i = 0; i < moduleSearchCache.length; i++) {
const item = moduleSearchCache[i].checkbox;
if (!evt) {
toggleClass(item.parentNode, 'checked', item.checked);
} else if (checkbox === allCheckbox && checkbox.checked) {
item.checked = false;
removeClass(item.parentNode, 'checked');
}
dirty = dirty || (item.checked !== item.defaultChecked);
if (item.checked) {
selectedNames.push(item.parentNode.textContent);
if (checkbox) {
// Update internal state
if (checkbox.checked) {
dropdownData.selectedMap.set(checkbox.value, checkbox.parentNode.textContent);
} else {
dropdownData.selectedMap.delete(checkbox.value);
}

// Update UI state
toggleClass(checkbox.parentNode, 'checked', checkbox.checked);
}

commit.style.display = reset.style.display = dirty ? '' : 'none';
moduleSearch.placeholder = selectedNames.join(', ') ||
allCheckbox.parentNode.textContent;
moduleSearch.title = 'Type to filter list. Current selection:\n' +
(selectedNames.join('\n') || allCheckbox.parentNode.textContent);
const textForm = dropdownData.selectedMap.size
? (dropdownData.selectedMap.size + ' ' + (dropdownData.selectedMap.size === 1 ? 'module' : 'modules'))
: 'All modules';
moduleSearch.placeholder = textForm;
moduleSearch.title = 'Type to search through and reduce the list.';
resetButton.disabled = !dropdownData.isDirty();
clearButton.style.display = dropdownData.selectedMap.size ? '' : 'none';
}

return moduleFilter;
Expand Down
Loading

0 comments on commit 06c3951

Please sign in to comment.