Skip to content

Commit

Permalink
fix: select first item when switching tags (#2026)
Browse files Browse the repository at this point in the history
  • Loading branch information
moughxyz committed Nov 17, 2022
1 parent e6baf74 commit b9e6dc4
Show file tree
Hide file tree
Showing 3 changed files with 132 additions and 19 deletions.
@@ -0,0 +1,106 @@
import { SNTag } from '@standardnotes/snjs'
import { ContentType } from '@standardnotes/common'
import { InternalEventBus } from '@standardnotes/services'
import { WebApplication } from '@/Application/Application'
import { LinkingController } from '../LinkingController'
import { NavigationController } from '../Navigation/NavigationController'
import { NotesController } from '../NotesController'
import { SearchOptionsController } from '../SearchOptionsController'
import { SelectedItemsController } from '../SelectedItemsController'
import { ItemListController } from './ItemListController'
import { ItemsReloadSource } from './ItemsReloadSource'

describe('item list controller', () => {
let controller: ItemListController
let navigationController: NavigationController
let selectionController: SelectedItemsController

beforeEach(() => {
const application = {} as jest.Mocked<WebApplication>
application.streamItems = jest.fn()
application.addEventObserver = jest.fn()
application.addWebEventObserver = jest.fn()

navigationController = {} as jest.Mocked<NavigationController>
selectionController = {} as jest.Mocked<SelectedItemsController>

const searchOptionsController = {} as jest.Mocked<SearchOptionsController>
const notesController = {} as jest.Mocked<NotesController>
const linkingController = {} as jest.Mocked<LinkingController>
const eventBus = new InternalEventBus()

controller = new ItemListController(
application,
navigationController,
searchOptionsController,
selectionController,
notesController,
linkingController,
eventBus,
)
})

describe('shouldSelectFirstItem', () => {
beforeEach(() => {
controller.getFirstNonProtectedItem = jest.fn()

Object.defineProperty(selectionController, 'selectedUuids', {
get: () => new Set(),
configurable: true,
})
})

it('should return false first item is file', () => {
controller.getFirstNonProtectedItem = jest.fn().mockReturnValue({
content_type: ContentType.File,
})

expect(controller.shouldSelectFirstItem(ItemsReloadSource.UserTriggeredTagChange)).toBe(false)
})

it('should return false if selected tag is daily entry', () => {
const tag = {
isDailyEntry: true,
content_type: ContentType.Tag,
} as jest.Mocked<SNTag>

Object.defineProperty(navigationController, 'selected', {
get: () => tag,
})

expect(controller.shouldSelectFirstItem(ItemsReloadSource.UserTriggeredTagChange)).toBe(false)
})

it('should return true if user triggered tag change', () => {
const tag = {
content_type: ContentType.Tag,
} as jest.Mocked<SNTag>

Object.defineProperty(navigationController, 'selected', {
get: () => tag,
})

expect(controller.shouldSelectFirstItem(ItemsReloadSource.UserTriggeredTagChange)).toBe(true)
})

it('should return false if not user triggered tag change and there is an existing selected item', () => {
const tag = {
content_type: ContentType.Tag,
} as jest.Mocked<SNTag>

Object.defineProperty(selectionController, 'selectedUuids', {
get: () => new Set(['123']),
})

Object.defineProperty(navigationController, 'selected', {
get: () => tag,
})

expect(controller.shouldSelectFirstItem(ItemsReloadSource.ItemStream)).toBe(false)
})

it('should return true if there are no selected items, even if not user triggered', () => {
expect(controller.shouldSelectFirstItem(ItemsReloadSource.ItemStream)).toBe(true)
})
})
})
Expand Up @@ -38,22 +38,13 @@ import { log, LoggingDomain } from '@/Logging'
import { NoteViewController } from '@/Components/NoteView/Controller/NoteViewController'
import { FileViewController } from '@/Components/NoteView/Controller/FileViewController'
import { TemplateNoteViewAutofocusBehavior } from '@/Components/NoteView/Controller/TemplateNoteViewControllerOptions'
import { ItemsReloadSource } from './ItemsReloadSource'

const MinNoteCellHeight = 51.0
const DefaultListNumNotes = 20
const ElementIdSearchBar = 'search-bar'
const ElementIdScrollContainer = 'notes-scrollable'

enum ItemsReloadSource {
ItemStream,
SyncEvent,
DisplayOptionsChange,
Pagination,
TagChange,
UserTriggeredTagChange,
FilterTextChange,
}

export class ItemListController extends AbstractViewController implements InternalEventHandlerInterface {
completedFullSync = false
noteFilterText = ''
Expand Down Expand Up @@ -122,7 +113,7 @@ export class ItemListController extends AbstractViewController implements Intern
application.streamItems<SNTag>([ContentType.Tag, ContentType.SmartView], async ({ changed, inserted }) => {
const tags = [...changed, ...inserted]

const { didReloadItems } = await this.reloadDisplayPreferences()
const { didReloadItems } = await this.reloadDisplayPreferences({ userTriggered: false })
if (!didReloadItems) {
/** A tag could have changed its relationships, so we need to reload the filter */
this.reloadNotesDisplayOptions()
Expand All @@ -138,7 +129,7 @@ export class ItemListController extends AbstractViewController implements Intern

this.disposers.push(
application.addEventObserver(async () => {
void this.reloadDisplayPreferences()
void this.reloadDisplayPreferences({ userTriggered: false })
}, ApplicationEvent.PreferencesChanged),
)

Expand Down Expand Up @@ -427,11 +418,11 @@ export class ItemListController extends AbstractViewController implements Intern
return false
}

private shouldSelectActiveItem = (activeItem: SNNote | FileItem | undefined) => {
return activeItem && !this.selectionController.isItemSelected(activeItem)
private shouldSelectActiveItem = (activeItem: SNNote | FileItem) => {
return !this.selectionController.isItemSelected(activeItem)
}

private shouldSelectFirstItem = (itemsReloadSource: ItemsReloadSource) => {
shouldSelectFirstItem = (itemsReloadSource: ItemsReloadSource) => {
const item = this.getFirstNonProtectedItem()
if (item && isFile(item)) {
return false
Expand All @@ -445,6 +436,7 @@ export class ItemListController extends AbstractViewController implements Intern

const userChangedTag = itemsReloadSource === ItemsReloadSource.UserTriggeredTagChange
const hasNoSelectedItem = !this.selectionController.selectedUuids.size

return userChangedTag || hasNoSelectedItem
}

Expand All @@ -466,7 +458,7 @@ export class ItemListController extends AbstractViewController implements Intern
log(LoggingDomain.Selection, 'Selecting next item after closing active one')
this.selectionController.selectNextItem()
}
} else if (this.shouldSelectActiveItem(activeItem) && activeItem) {
} else if (activeItem && this.shouldSelectActiveItem(activeItem)) {
log(LoggingDomain.Selection, 'Selecting active item')
await this.selectionController.selectItem(activeItem.uuid).catch(console.error)
} else if (this.shouldSelectFirstItem(itemsReloadSource)) {
Expand Down Expand Up @@ -510,7 +502,11 @@ export class ItemListController extends AbstractViewController implements Intern
this.application.items.setPrimaryItemDisplayOptions(criteria)
}

reloadDisplayPreferences = async (): Promise<{ didReloadItems: boolean }> => {
reloadDisplayPreferences = async ({
userTriggered,
}: {
userTriggered: boolean
}): Promise<{ didReloadItems: boolean }> => {
const newDisplayOptions = {} as DisplayOptions
const newWebDisplayOptions = {} as WebDisplayOptions
const selectedTag = this.navigationController.selected
Expand Down Expand Up @@ -601,7 +597,9 @@ export class ItemListController extends AbstractViewController implements Intern

this.reloadNotesDisplayOptions()

await this.reloadItems(ItemsReloadSource.DisplayOptionsChange)
await this.reloadItems(
userTriggered ? ItemsReloadSource.UserTriggeredTagChange : ItemsReloadSource.DisplayOptionsChange,
)

const didSortByChange = currentSortBy !== this.displayOptions.sortBy
const didSortDirectionChange = currentSortDirection !== this.displayOptions.sortDirection
Expand Down Expand Up @@ -813,7 +811,7 @@ export class ItemListController extends AbstractViewController implements Intern

this.resetPagination()

const { didReloadItems } = await this.reloadDisplayPreferences()
const { didReloadItems } = await this.reloadDisplayPreferences({ userTriggered })

if (!didReloadItems) {
this.reloadNotesDisplayOptions()
Expand Down
@@ -0,0 +1,9 @@
export enum ItemsReloadSource {
ItemStream,
SyncEvent,
DisplayOptionsChange,
Pagination,
TagChange,
UserTriggeredTagChange,
FilterTextChange,
}

0 comments on commit b9e6dc4

Please sign in to comment.