Skip to content
This repository was archived by the owner on Dec 15, 2022. It is now read-only.

Improve initial sorting of items #87

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Improve initial sorting of items #87

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
333180e
add initial prototype of functionality
jarle Sep 25, 2017
370c97e
rework filter method a bit, scoped state better
jarle Sep 26, 2017
26d7c20
change settings to include more options
jarle Sep 26, 2017
3e9e2f4
add options for sorting by recent usage
jarle Sep 26, 2017
d72748d
fix small bug and revert item titles
jarle Sep 26, 2017
f808337
started working on tests, added third option for alphabetic sorting
jarle Sep 29, 2017
d1da17b
added more test cases, broke out common code
jarle Sep 29, 2017
98acb8e
implemented tests for serializing view, fixed alphabetic assertion
jarle Sep 29, 2017
016f317
fewer mock commands used in tests to avoid timeout
jarle Sep 30, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions lib/command-palette-package.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ import {CompositeDisposable} from 'atom'
import CommandPaletteView from './command-palette-view'

class CommandPalettePackage {
activate () {
this.commandPaletteView = new CommandPaletteView()
activate (state) {
this.commandPaletteView = new CommandPaletteView(state)
this.disposables = new CompositeDisposable()
this.disposables.add(atom.commands.add('atom-workspace', 'command-palette:toggle', () => {
this.commandPaletteView.toggle()
Expand All @@ -16,9 +16,16 @@ class CommandPalettePackage {
this.disposables.add(atom.config.observe('command-palette.preserveLastSearch', (newValue) => {
this.commandPaletteView.update({preserveLastSearch: newValue})
}))
this.disposables.add(atom.config.observe('command-palette.initialOrderingOfItems', (newValue) => {
this.commandPaletteView.update({initialOrderingOfItems: newValue})
}))
return this.commandPaletteView.show()
}

serialize() {
return this.commandPaletteView.serialize()
}

async deactivate () {
this.disposables.dispose()
await this.commandPaletteView.destroy()
Expand Down
47 changes: 45 additions & 2 deletions lib/command-palette-view.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ import fuzzaldrin from 'fuzzaldrin'
import fuzzaldrinPlus from 'fuzzaldrin-plus'

export default class CommandPaletteView {
constructor () {
constructor (state = {}) {
this.itemLaunches = state.itemLaunches || {}
this.keyBindingsForActiveElement = []
this.commandsForActiveElement = []
this.selectListView = new SelectListView({
Expand Down Expand Up @@ -69,6 +70,14 @@ export default class CommandPaletteView {
},
didConfirmSelection: (keyBinding) => {
this.hide()
const elementName = keyBinding.name
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could use a better name. elementName makes it seem as though it's referring to the DOM.

const launchedAt = new Date().getTime()
if(this.itemLaunches.hasOwnProperty(elementName)) {
this.itemLaunches[elementName].push(launchedAt)
}
else {
this.itemLaunches[elementName] = [launchedAt]
}
const event = new CustomEvent(keyBinding.name, {bubbles: true, cancelable: true})
this.activeElement.dispatchEvent(event)
},
Expand Down Expand Up @@ -130,6 +139,10 @@ export default class CommandPaletteView {
if (props.hasOwnProperty('useAlternateScoring')) {
this.useAlternateScoring = props.useAlternateScoring
}

if (props.hasOwnProperty('initialOrderingOfItems')) {
this.initialOrderingOfItems = props.initialOrderingOfItems
}
}

get fuzz () {
Expand Down Expand Up @@ -171,9 +184,39 @@ export default class CommandPaletteView {
}
}

serialize = () => ({
itemLaunches: this.itemLaunches
})

filter = (items, query) => {
if (query.length === 0) {
return items
if (Object.keys(this.itemLaunches).length === 0) return items;
if (this.initialOrderingOfItems === 'alphabetic') return items;

const scoredItems = []
const unscoredItems = []

for (const item of items) {
const launchDates = this.itemLaunches[item.name] || []
let score;
if (this.initialOrderingOfItems === 'frequency') {
score = launchDates.length
}
else if(this.initialOrderingOfItems === 'recent') {
score = launchDates[launchDates.length-1]
}

if(score) {
scoredItems.push({item, score})
}
else {
unscoredItems.push(item)
}
}
return scoredItems
.sort((a, b) => b.score - a.score)
.map(i => i.item)
.concat(unscoredItems)
}

const scoredItems = []
Expand Down
10 changes: 10 additions & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,16 @@
"type": "boolean",
"default": false,
"description": "Preserve the last search when reopening the command palette."
},
"initialOrderingOfItems": {
"type": "string",
"default": "frequency",
"enum": [
{"value": "frequency", "description": "By frequency of use"},
{"value": "recent", "description": "By most recently used"},
{"value": "alphabetic", "description": "Alphabetical order"}
]

}
}
}
194 changes: 194 additions & 0 deletions test/command-palette-view.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,200 @@ describe('CommandPaletteView', () => {
workspaceElement.remove()
})

describe('initial sorting', () => {
let commandPalette
const fakeCommands = [
'one',
'two',
'three',
].map(command => `command-palette-test:${command}`)

beforeEach(async () => {
commandPalette = new CommandPaletteView()
for (let i=0; i<fakeCommands.length; i++) {
const command = {
name: fakeCommands[i],
displayName: fakeCommands[i].replace(/-/g, ' ')
}
atom.commands.add('atom-workspace', command.name, () => {})

const numTimesToLaunch = fakeCommands.length - i
//console.log(`Launching ${command.displayName} ${numTimesToLaunch} times`)

for (j=0; j<numTimesToLaunch; j++) {
await commandPalette.show()
await commandPalette.selectListView.refs.queryEditor.setText(command.displayName)
assert.equal(commandPalette.selectListView.getSelectedItem().name, command.name)
await commandPalette.selectListView.confirmSelection()
}
}
})

describe('when initially sorting by frequency', () => {
beforeEach(async () => {
await commandPalette.update({initialOrderingOfItems: 'frequency'})
})

it('orders the scored items correctly', async () => {
await commandPalette.show()
await commandPalette.selectListView.refs.queryEditor.setText('')
await commandPalette.selectListView.update()

fakeCommands.forEach(command => {
const selectedItem = commandPalette.selectListView.getSelectedItem().name
assert.equal(selectedItem, command)
commandPalette.selectListView.selectNext()
})
})

it('orders the rest of the palette items alphabetically', async () => {
await commandPalette.show()
await commandPalette.selectListView.refs.queryEditor.setText('')
await commandPalette.selectListView.update()

commandPalette.selectListView.selectFirst()
const firstItem = commandPalette.selectListView.getSelectedItem().name

// skip scored items
for(let i=0; i<fakeCommands.length; i++) { commandPalette.selectListView.selectNext() }

// compare pairwise items
let currentItem, previousItem
do {
previousItem = commandPalette.selectListView.getSelectedItem().name
commandPalette.selectListView.selectNext()
currentItem = commandPalette.selectListView.getSelectedItem().name
if(currentItem == firstItem) break;
assert.equal(previousItem.localeCompare(currentItem), -1)
}
while (previousItem != firstItem)
})

it('remembers the ordering between launches', async () => {
const serializedState = commandPalette.serialize();
const newCommandPalette = new CommandPaletteView(serializedState);

await newCommandPalette.update({initialOrderingOfItems: 'frequency'})
await newCommandPalette.show()
await newCommandPalette.selectListView.refs.queryEditor.setText('')
await newCommandPalette.selectListView.update()

fakeCommands.forEach(command => {
const selectedItem = newCommandPalette.selectListView.getSelectedItem().name
assert.equal(selectedItem, command)
newCommandPalette.selectListView.selectNext()
})
})
})

describe('when initially sorting by recentness', () => {
beforeEach(async () => {
await commandPalette.update({initialOrderingOfItems: 'recent'})
})

it('orders the scored items correctly', async () => {
await commandPalette.show()
await commandPalette.selectListView.refs.queryEditor.setText('')
await commandPalette.selectListView.update()

fakeCommands.reverse().forEach(command => {
const selectedItem = commandPalette.selectListView.getSelectedItem().name
assert.equal(selectedItem, command)
commandPalette.selectListView.selectNext()
})
})

it('orders the rest of the palette items alphabetically', async () => {
await commandPalette.show()
await commandPalette.selectListView.refs.queryEditor.setText('')
await commandPalette.selectListView.update()

commandPalette.selectListView.selectFirst()
const firstItem = commandPalette.selectListView.getSelectedItem().name

// skip scored items
for(let i=0; i<fakeCommands.length; i++) { commandPalette.selectListView.selectNext() }

// compare pairwise items
let currentItem, previousItem
do {
previousItem = commandPalette.selectListView.getSelectedItem().name
commandPalette.selectListView.selectNext()
currentItem = commandPalette.selectListView.getSelectedItem().name
if(currentItem == firstItem) break;
assert.equal(previousItem.localeCompare(currentItem), -1)
}
while (previousItem != firstItem)
})

it('remembers the ordering between launches', async () => {
const serializedState = commandPalette.serialize();
const newCommandPalette = new CommandPaletteView(serializedState);

await newCommandPalette.update({initialOrderingOfItems: 'recent'})
await newCommandPalette.show()
await newCommandPalette.selectListView.refs.queryEditor.setText('')
await newCommandPalette.selectListView.update()

fakeCommands.reverse().forEach(command => {
const selectedItem = newCommandPalette.selectListView.getSelectedItem().name
assert.equal(selectedItem, command)
newCommandPalette.selectListView.selectNext()
})
})
})

describe('when initially sorting alphabetically', () => {
beforeEach(async () => {
await commandPalette.update({initialOrderingOfItems: 'alphabetic'})
})

it('orders the palette items correctly', async () => {
await commandPalette.show()
await commandPalette.selectListView.refs.queryEditor.setText('')
await commandPalette.selectListView.update()

commandPalette.selectListView.selectFirst()
const firstItem = commandPalette.selectListView.getSelectedItem().name

// compare pairwise items
let currentItem, previousItem
do {
previousItem = commandPalette.selectListView.getSelectedItem().name
commandPalette.selectListView.selectNext()
currentItem = commandPalette.selectListView.getSelectedItem().name
if(currentItem == firstItem) break;
assert.equal(previousItem.localeCompare(currentItem), -1)
}
while (previousItem != firstItem)
})

it('remembers the ordering between launches', async () => {
const serializedState = commandPalette.serialize();
const newCommandPalette = new CommandPaletteView(serializedState);

await newCommandPalette.update({initialOrderingOfItems: 'recent'})
await newCommandPalette.show()
await newCommandPalette.selectListView.refs.queryEditor.setText('')
await newCommandPalette.selectListView.update()

commandPalette.selectListView.selectFirst()
const firstItem = commandPalette.selectListView.getSelectedItem().name

// compare pairwise items
let currentItem, previousItem
do {
previousItem = commandPalette.selectListView.getSelectedItem().name
commandPalette.selectListView.selectNext()
currentItem = commandPalette.selectListView.getSelectedItem().name
if(currentItem == firstItem) break;
assert.equal(previousItem.localeCompare(currentItem), -1)
}
while (previousItem != firstItem)
})
})
})

describe('toggle', () => {
describe('when an element is focused', () => {
it('shows a list of all valid command descriptions, names, and keybindings for the previously focused element', async () => {
Expand Down