Skip to content

Commit

Permalink
🚸 Assorted User Experience fixes (#51)
Browse files Browse the repository at this point in the history
  • Loading branch information
trickypr authored Jan 15, 2024
1 parent 5ed7adf commit e5cdfbf
Show file tree
Hide file tree
Showing 14 changed files with 181 additions and 70 deletions.
3 changes: 2 additions & 1 deletion apps/content/src/browser/components/Browser.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@
display: none;
}
:global(browser) {
.browser-container :global(browser) {
flex-grow: 1;
color-scheme: env(-moz-content-preferred-color-scheme);
}
</style>
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@
pref="browser.keybinds.closeTab"
on:command={() => runOnCurrentTab(closeTab)}
/>
<Keybinding
pref="browser.keybinds.refreshTab"
on:command={() => runOnCurrentTab((tab) => tab.reload())}
/>
<Keybinding
pref="browser.keybinds.nextTab"
on:command={() => setCurrentTabIndex(getCurrentTabIndex() + 1)}
Expand Down
55 changes: 35 additions & 20 deletions apps/content/src/browser/components/omnibox/Omnibox.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -16,40 +16,49 @@
export let tab: Tab
let showFocus = false
let inputContent: string = ''
let inputElement: HTMLInputElement
let suggestions: Suggestion[] = []
let selectedSuggestion = 0
$: focusedOmnibox = tab.focusedOmnibox
$: uri = tab.uri
$: zoom = tab.zoom
async function generateSuggestions() {
const { suggestions: suggestionsMethod } = await suggestionsModule
suggestions = await suggestionsMethod(inputContent)
const setSuggestions = (query: string) => (s: Suggestion[]) => {
if (query != inputContent) return
suggestions = s
selectedSuggestion = Math.max(
Math.min(selectedSuggestion, suggestions.length - 1),
0,
)
}
const unbindedSetInputContent = (value: string) => {
inputContent = value
if (tab.tabJustOpened && inputElement) {
inputElement.focus()
inputElement.select()
tab.tabJustOpened = false
return
}
async function generateSuggestions() {
const { suggestions: suggestionsMethod } = await suggestionsModule
const { fast, full } = suggestionsMethod(inputContent)
fast.then(setSuggestions(inputContent))
full.then(setSuggestions(inputContent))
}
function updateFocus(shouldBeFocused: boolean, url: string) {
inputContent = url
if (!inputElement) return
// Unfocus on spec change
if (inputElement && suggestions.length != 0) {
const isFocused = document.activeElement === inputElement
if (isFocused === shouldBeFocused) return
if (shouldBeFocused) {
inputElement.focus()
setTimeout(() => inputElement.select(), 100)
} else {
inputElement.blur()
suggestions = []
}
}
$: unbindedSetInputContent($uri.asciiSpec)
$: updateFocus($focusedOmnibox, $uri.asciiSpec)
</script>

<div class="container">
Expand All @@ -63,16 +72,22 @@
bind:this={inputElement}
bind:value={inputContent}
on:focusin={() => {
showFocus = true
focusedOmnibox.set(true)
generateSuggestions()
}}
on:blur|capture={() =>
setTimeout(() => (showFocus = false) && (suggestions = []), 100)}
setTimeout(() => {
focusedOmnibox.set(false)
suggestions = []
}, 100)}
on:keyup={async (e) => {
if (e.key === 'Enter')
if (e.key === 'Enter') {
focusedOmnibox.set(false)
return tab.goToUri(
resource.NetUtil.newURI(suggestions[selectedSuggestion].url),
)
}

if (e.key === 'ArrowDown') {
e.preventDefault()
selectedSuggestion = Math.min(
Expand Down Expand Up @@ -104,7 +119,7 @@

<div
class="suggestions"
hidden={!showFocus}
hidden={!$focusedOmnibox}
id="omnibox__suggestions-list"
role="listbox"
>
Expand All @@ -119,7 +134,7 @@
aria-selected={index === selectedSuggestion}
on:click={(_) => {
tab.goToUri(resource.NetUtil.newURI(suggestion.url))
inputElement.blur()
focusedOmnibox.set(false)
}}
>
{suggestion.title}
Expand Down
11 changes: 1 addition & 10 deletions apps/content/src/browser/lib/window/tab.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,7 @@ export class Tab {

public zoom = writable(1)

/**
* This is used by the omnibox to determine if text input should be focused.
*/
public tabJustOpened = true

public focusedOmnibox = writable(true)
public hidden = writable(false)

constructor(uri: nsIURIType) {
Expand Down Expand Up @@ -143,13 +139,10 @@ export class Tab {
'pagetitlechanged',
this.onPageTitleChanged.bind(this),
)

this.browserElement.removeEventListener(
'DidChangeBrowserRemoteness',
this.onDidChangeBrowserRemoteness.bind(this),
)

this.progressListener.remove(this.browserElement)
}

protected onPageTitleChanged() {
Expand Down Expand Up @@ -203,9 +196,7 @@ export class Tab {
}

public destroy() {
this.removeEventListeners()
resource.ZoomStore.events.off('setZoom', this.zoomChange)

this.browserElement.remove()
}

Expand Down
1 change: 1 addition & 0 deletions apps/content/src/settings/Settings.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@
<SubCategory title="Tabs & Websites">
<StringPref pref="browser.keybinds.newWindow">New Window</StringPref>
<StringPref pref="browser.keybinds.newTab">New Tab</StringPref>
<StringPref pref="browser.keybinds.refreshTab">Refresh Tab</StringPref>
<StringPref pref="browser.keybinds.closeTab">Close Active Tab</StringPref>
<StringPref pref="browser.keybinds.nextTab">Next tab</StringPref>
<StringPref pref="browser.keybinds.previousTab">Previous Tab</StringPref>
Expand Down
8 changes: 8 additions & 0 deletions apps/content/src/shared/search/provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,5 +35,13 @@ export abstract class Provider {
*/
public abstract providerPriority: number

/**
* Should return results that do not require network queries. Your highest
* priority results should be returned from this method
*/
public abstract getFastResults(
query: string,
): Promise<ProviderResult[]> | ProviderResult[]

public abstract getResults(query: string): Promise<ProviderResult[]>
}
23 changes: 14 additions & 9 deletions apps/content/src/shared/search/providers/EngineProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,17 @@ export class EngineProvider extends Provider {
return submission.uri
}

async getFastResults(query: string) {
const engine = await this.engine.promise
const defaultSearchResult: ProviderResult = {
title: query,
url: this.getSearchUri(engine, query).spec,
priority: ResultPriority.HIGH,
}

return [defaultSearchResult]
}

async getResults(query: string): Promise<ProviderResult[]> {
if (query == '' || isUrlLike(query)) return []

Expand All @@ -46,17 +57,11 @@ export class EngineProvider extends Provider {
const response = await request
const body = await response.text()

const defaultSearchResult: ProviderResult = {
title: query,
url: this.getSearchUri(engine, query).spec,
priority: ResultPriority.HIGH,
}

if (response.status != 200) {
console.error(
`Search engine ${engine.name} returned status ${response.status}`,
)
return [defaultSearchResult]
return []
}

try {
Expand All @@ -69,12 +74,12 @@ export class EngineProvider extends Provider {
priority: ResultPriority.LOW,
}
})
return [defaultSearchResult, ...results]
return [...results]
} catch (e) {
console.error(
`Search engine ${engine.name} returned invalid JSON: ${body}`,
)
return [defaultSearchResult]
return []
}
}
}
6 changes: 5 additions & 1 deletion apps/content/src/shared/search/providers/URLProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,11 @@ export function isUrlLike(query: string): boolean {
export class URLProvider extends Provider {
public providerPriority = 0

public async getResults(query: string): Promise<ProviderResult[]> {
public async getResults(_query: string) {
return []
}

public getFastResults(query: string): ProviderResult[] {
// Check against chrome urls
if (
CHROME_REGEX.test(query) ||
Expand Down
37 changes: 27 additions & 10 deletions apps/content/src/shared/search/suggestions.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
import type { Provider } from './provider'
import type { Provider, ProviderResult } from './provider'
import { EngineProvider, URLProvider } from './providers'

/**
Expand All @@ -17,22 +17,39 @@ export interface Suggestion {
url: string
}

const take =
(n: number) =>
<T>(_: T, i: number) =>
i < n

function processResults(providerResults: ProviderResult[][]): Suggestion[] {
return providerResults
.flat()
.sort((a, b) => a.priority - b.priority)
.filter(take(5))
.map(({ title, icon, url }) => ({ title, icon, url }))
}

/**
* Generates a list of autocomplete suggestions for the user's input
* @param query The user's input to perform prediction on
* @returns The suggestions for the user's input
*/
export async function suggestions(query: string): Promise<Suggestion[]> {
const results = await Promise.all(
export function suggestions(query: string): {
fast: Promise<Suggestion[]>
full: Promise<Suggestion[]>
} {
const fastResultsPromise = Promise.all(
PROVIDERS.map((provider) => provider.getFastResults(query)),
)
const fullResultsPromise = Promise.all(
PROVIDERS.map((provider) => provider.getResults(query)),
)

return (
results
.flat()
// We want to sort by priority from low to high
.sort((a, b) => a.priority - b.priority)
.filter((_, index) => index < 5)
.map(({ title, icon, url }) => ({ title, icon, url }))
const fastSuggestions = fastResultsPromise.then(processResults)
const fullSuggestions = fullResultsPromise.then(async (results) =>
processResults([...(await fastResultsPromise), ...results]),
)

return { fast: fastSuggestions, full: fullSuggestions }
}
27 changes: 26 additions & 1 deletion apps/content/src/tests/manager.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
import { createTAPReporter, hold, report } from 'zora'
import {
type IAssert,
type ISpecFunction,
createTAPReporter,
hold,
report,
} from 'zora'

const TEST_PORT = 3948
const TEST_OUTPUT = document.createElement('pre')
Expand Down Expand Up @@ -45,3 +51,22 @@ export async function manageTests(

return performTests
}

interface IAssertionResult<T> {
pass: boolean
actual: unknown
expected: T
description: string
operator: string
at?: string
}

export async function then<T>(
t: IAssert,
result: IAssertionResult<T>,
description: string,
fn: ISpecFunction,
) {
const restFn = result.pass ? t.test : t.skip
restFn(description, fn)
}
25 changes: 23 additions & 2 deletions apps/content/src/tests/shared/search/providers/EngineProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import { searchEngineService } from '@shared/search/engine'
import { ResultPriority } from '@shared/search/provider'
import { EngineProvider } from '@shared/search/providers'

import { then } from '../../../manager'

export default async function () {
await test('EngineProvider: ignore URL like', async (t) => {
const provider = new EngineProvider()
Expand All @@ -32,8 +34,27 @@ export default async function () {
const results = await provider.getResults(
'cc4ce6fc-c166-4501-b34f-bda93579efc2',
)
t.eq(results.length, 1, 'Should have a single result')
t.eq(results[0].priority, ResultPriority.HIGH, 'Should have high priority')
t.eq(results.length, 0, 'Should have a single result')
})

await test('EngineProvider: fast recommendations', async (t) => {
const provider = new EngineProvider()
const results = await provider.getFastResults('tes')

then(
t,
t.equals(results.length, 1, 'should have a single result'),

'validate results',
(t) => {
t.equals(
results[0].priority,
ResultPriority.HIGH,
'should have a high priority',
)
t.equals(results[0].title, 'tes', 'should have the same title')
},
)
})

for (const engine of SEARCH_ENGINE_IDS) {
Expand Down
Loading

0 comments on commit e5cdfbf

Please sign in to comment.