Add sorting bar to note list

[codebykat]

Fix

Adds a "Sort by" bar to the bottom of the notes list. See p2XJRt-2Ct-p2

Designs are at:
https://app.zeplin.io/project/5d795a3d8ed5261aa146e057/screen/5fd77f1846903987b1b40e75 (light mode)
https://app.zeplin.io/project/5d795a3d8ed5261aa146e057/screen/5fd77f196e43637c2cb2889f (dark mode)

Also shortens the search results bar to 44px to match the height of the sorting bar and the new toolbar-height.

Fixes #1625

Test

1. Use the toggle to change the sort type and direction of the results.
2. Also try it with search filtering
3. Try in different browsers/OSes to make sure the dropdown styling is okay

Screenshots

Note that in all browsers except Firefox, the dropdown styling is determined by the system theme, so it looks like this if you have a light system theme and the dark Simplenote theme:

In Firefox, we have to style the dropdown since otherwise it takes the styles from the parent, so I have added dark mode styles for Firefox that will be shown regardless of the system setting (in Simplenote dark theme):

The weird thing is that setting a background on the select seems to wipe out the padding and rounded edges (only in dark mode), but if we don't set it, we get this:

Firefox light theme changes the white background to a light gray, which matches default styling:

Search results bar shortened to match:

Release

Added sort order bar to note list

[dmsnell]

Looks fine as it is but I left a few comments that you might consider. Feel free to merge it after responding to or rejecting my suggestions.

[codebykat]

Next steps on this:

1. Change dropdown labels per new designs (p2XJRt-2Ct-p2) 3a9fa0e

all 6 sorting options should be listed within each platforms ‘dropdown’ menu to reduce visual prominence of the bar since it will now be displayed all the time.

Dropdown should be:
Name: A-Z
Name: Z-A
Created: Newest
Created: Oldest
Modified: Newest
Modified: Oldest

2. Add tracks event cb083c9, 3c28668

sp*_list_sortbar_mode_changed

3. Address review feedback 8c26946

[codebykat]

Refactored with some data attributes, thanks @dmsnell for the suggestion.

[SylvesterWilmott]

The design looks good! Thanks @codebykat

[dmsnell]

this is confusing me, specifically because we're comparing against the string value of false

I think we're doing it funny-like because it's a string value and not a real value. in this case parsing it could go a long way for readability. what I expect is to find something like selectedSortReversed !== sortReversed

const selectedSortReversed = selectedOption.dataset.reversed === 'true'; // true if 'true' else false
const selectedSortReversed = JSON.parse(selectedOption.dataset.reversed); // 'true' and 'false' decode properly

if (selectedSortReversed !== sortReversed) {
	…
}

[codebykat]

Oops, I should have refactored that out actually.

[dmsnell]

did you find HTMLSelectElement.selectedIndex?

[codebykat]

Yes but is dropdown.options[selectedIndex] better in some way than dropdown.selectedOptions[0]?

[dmsnell]

only in the sense that it relies on named things vs. the assumption of 0. I know that I wouldn't expect the selected option to always be the first one in the list, or that's how I read this line. I'm guessing this is a list of all possible selections because a drop down can have multiple selected items?

otherwise we're getting the actual option I think, if using seletedIndex

[dmsnell]

if we want to simplify the flow of data and types through the select component then we can do another thing and that is directly index into this list. consider if this sortTypes array were outside of the component and in the module scope and we had a function to find the current setting. our HTMLSelectElement could refer to an index into the array as the choice, and although that's less meaningful in itself, it does preserve the types the eliminates the processing and parsing needed to re-connect the data.

type SortOption = {
	label: string,
	order: T.SortOrder,
	isReversed: boolean
}

const sortTypes: SortOption[] = [{
	label: 'Name: A-Z',
  order: 'alphabetical',
  isReversed: false
}, {
	…
}];

const onChange = ({currentTarget: {selectedIndex}}) => {
	if (-1 === selectedIndex) {
		return;
	}

	const sort = sortTypes[selectedIndex];
	if (sort.sortOrder !== sortOrder) {
		…
	}

	if (sort.isReversed !== sortReversed) {
		…
	}
}

const sortTypesIndex = (sortOrder, isReversed) => sortTypes.findIndex(type => type.order === sortOrder && type.isReversed === isReversed);

<select id="sort-selection" value={sortTypesIndex(sortOrder, reversed)} onChange={…}>
{sortTypes.map((type, index) => (
	<option key={type.label} value={index}>{type.label}</option>
))}
</select>

[dmsnell]

it's rare to use an index as the React key but here since the sortTypes array is static the indices will never change and it's safe to use that value

[codebykat]

That works like a charm! a7d6f62

[dmsnell]

calling two separate Redux actions introduces another partial update into our app. that is, if we change both sort properties then we're asking React to update after the first Redux dispatch and then again after the second. that intervening time represents a state that was never requested by the customer.

should we examine the setSortType action and give it the ability to also change the reversing of the search? if we did that, add an optional reversed: boolean property to the action, then we could perform the update in one go and we could eliminate the conditionals at the same time.

[codebykat]

Very cool! I do end up checking for undefined a bit, not sure if that's the right way to do this. 3e8bac4

[dmsnell]

the logic is more readable to me after using the array of sort options vs. the data-passing. good detail work making sure the search index updates ✅

[pachlava]

[testing] fix/feature verified on Dell Latitude 7400 | Win 10 | Simplenote 2.7.0-beta1