From 0af379db7d5c2bbf4f8297fc629211f09cca5292 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaros=C5=82aw=20Sadzi=C5=84ski?= Date: Tue, 27 Sep 2022 23:06:02 +0200 Subject: [PATCH] (core) Fixing UserManger releted tests Summary: Some tests were not compatible with the new ACUser search component. Test Plan: Existing Reviewers: georgegevoian Reviewed By: georgegevoian Differential Revision: https://phab.getgrist.com/D3643 --- app/client/lib/ACUserManager.ts | 69 ++++++++++++--------------- app/client/lib/autocomplete.ts | 9 ++-- app/client/models/UserManagerModel.ts | 3 +- app/client/ui/UserManager.ts | 19 ++------ 4 files changed, 43 insertions(+), 57 deletions(-) diff --git a/app/client/lib/ACUserManager.ts b/app/client/lib/ACUserManager.ts index f97a1628..cb006125 100644 --- a/app/client/lib/ACUserManager.ts +++ b/app/client/lib/ACUserManager.ts @@ -1,11 +1,8 @@ -import {ACResults, ACIndex, ACItem, buildHighlightedDom, normalizeText} from "app/client/lib/ACIndex"; +import {ACIndex, ACItem, ACResults, buildHighlightedDom, normalizeText} from "app/client/lib/ACIndex"; import {cssSelectItem} from "app/client/lib/ACSelect"; import {Autocomplete, IAutocompleteOptions} from "app/client/lib/autocomplete"; -import {cssMenuItem} from "popweasel"; - -import {testId, colors, theme} from "app/client/ui2018/cssVars"; +import {colors, testId, theme} from "app/client/ui2018/cssVars"; import {menuCssClass} from "app/client/ui2018/menus"; -import {dom, DomElementArg, Holder, IDisposableOwner, Observable, styled, computed, Computed} from "grainjs"; import { cssEmailInput, cssEmailInputContainer, @@ -17,6 +14,8 @@ import { cssMemberText, } from "app/client/ui/UserItem"; import {createUserImage, cssUserImage} from "app/client/ui/UserImage"; +import {Computed, computed, dom, DomElementArg, Holder, IDisposableOwner, Observable, styled} from "grainjs"; +import {cssMenuItem} from "popweasel"; export interface ACUserItem extends ACItem { value: string; @@ -33,16 +32,17 @@ export function buildACMemberEmail( options: { acIndex: ACIndex; emailObs: Observable; - save: (value: string) => Promise | void; - isInputValid: Observable; + save: (value: string) => void; prompt?: {email: string}, }, ...args: DomElementArg[] ) { - const { acIndex, emailObs, save, isInputValid, prompt } = options; + const {acIndex, emailObs, save, prompt} = options; const acHolder = Holder.create>(owner); let emailInput: HTMLInputElement; + const isValid = Observable.create(owner, true); + const isOpen = () => !acHolder.isEmpty(); const acOpen = () => acHolder.isEmpty() && Autocomplete.create(acHolder, emailInput, acOptions); const acClose = () => acHolder.clear(); @@ -52,7 +52,7 @@ export function buildACMemberEmail( emailInput.value = emailObs.get(); emailInput.focus(); }; - const openOrCommit = () => { + const onEnter = () => { isOpen() ? commitIfValid() : acOpen(); }; @@ -62,14 +62,16 @@ export function buildACMemberEmail( emailObs.set(item.value); } emailInput.setCustomValidity(""); + isValid.set(emailInput.checkValidity()); + const selectedEmail = item?.value || emailObs.get(); try { - if (selectedEmail && isInputValid.get()) { + if (selectedEmail && isValid.get()) { save(emailObs.get()); finish(); } } catch (e) { - emailInput.setCustomValidity(e.message); + emailInput.setCustomValidity(e.message); } finally { emailInput.reportValidity(); } @@ -79,7 +81,7 @@ export function buildACMemberEmail( const cleanText = normalizeText(text); const items = results.items .filter(item => item.cleanText.includes(cleanText)) - .sort((a,b) => a.cleanText.localeCompare(b.cleanText)); + .sort((a, b) => a.cleanText.localeCompare(b.cleanText)); results.items = items; if (!results.items.length) { const newObject = { @@ -102,7 +104,7 @@ export function buildACMemberEmail( "+", cssUserImage.cls("-large"), cssUserImagePlus.cls('-invalid', (use) => !use(enableAdd), - )), + )), cssMemberText( cssMemberPrimaryPlus("Invite new member"), cssMemberSecondaryPlus( @@ -121,7 +123,7 @@ export function buildACMemberEmail( ) )); - const enableAdd: Computed = computed((use) => Boolean(use(emailObs) && use(isInputValid))); + const enableAdd: Computed = computed((use) => Boolean(use(emailObs) && use(isValid))); const acOptions: IAutocompleteOptions = { attach: null, @@ -136,7 +138,7 @@ export function buildACMemberEmail( cssMailIcon("Mail"), (emailInput = cssEmailInput( emailObs, - {onInput: true, isValid: isInputValid}, + {onInput: true, isValid}, {type: "email", placeholder: "Enter email address"}, dom.on("input", acOpen), dom.on("focus", acOpen), @@ -144,51 +146,43 @@ export function buildACMemberEmail( dom.on("blur", acClose), dom.onKeyDown({ Escape: finish, - Enter: openOrCommit, + Enter: onEnter, ArrowDown: acOpen, Tab: commitIfValid, }), - ...args - )), + )), cssEmailInputContainer.cls('-green', enableAdd), + ...args ); + // Reset custom validity that we sometimes set. + owner.autoDispose(emailObs.addListener(() => emailInput.setCustomValidity(""))); + if (prompt) { setTimeout(() => emailInput.focus(), 0); } return result; } -const cssMemberPrimaryPlus = styled( - cssMemberPrimary, - ` +const cssMemberPrimaryPlus = styled(cssMemberPrimary, ` .${cssSelectItem.className}.selected & { color: ${theme.menuItemSelectedFg}; } -` -); +`); -const cssMemberSecondaryPlus = styled( - cssMemberSecondary, - ` +const cssMemberSecondaryPlus = styled(cssMemberSecondary, ` .${cssSelectItem.className}.selected & { color: ${theme.menuItemSelectedFg}; } -` -); +`); -const cssMatchText = styled( - "span", - ` +const cssMatchText = styled("span", ` color: ${theme.autocompleteMatchText}; .${cssSelectItem.className}.selected & { color: ${theme.autocompleteSelectedMatchText}; } -` -); +`); -const cssUserImagePlus = styled( - cssUserImage, - ` +const cssUserImagePlus = styled(cssUserImage, ` background-color: ${colors.lightGreen}; margin: auto 0; @@ -200,5 +194,4 @@ const cssUserImagePlus = styled( background-color: ${theme.menuItemIconSelectedFg}; color: ${theme.menuItemSelectedBg}; } -` -); +`); diff --git a/app/client/lib/autocomplete.ts b/app/client/lib/autocomplete.ts index d9d1c078..12ef6608 100644 --- a/app/client/lib/autocomplete.ts +++ b/app/client/lib/autocomplete.ts @@ -21,6 +21,11 @@ export interface IAutocompleteOptions { // Popper options for positioning the popup. popperOptions?: Partial; + // To which element to append the popup content. Null means triggerElem.parentNode; string is + // a selector for the closest matching ancestor of triggerElem, e.g. 'body'. + // Defaults to the document body. + attach?: Element|string|null; + // Given a search term, return the list of Items to render. search(searchText: string): Promise>; @@ -32,10 +37,6 @@ export interface IAutocompleteOptions { // A callback triggered when user clicks one of the choices. onClick?(): void; - - // To which element to append the popup content. Null means triggerElem.parentNode and is the - // default; string is a selector for the closest matching ancestor of triggerElem, e.g. 'body'. - attach?: Element|string|null; } /** diff --git a/app/client/models/UserManagerModel.ts b/app/client/models/UserManagerModel.ts index 22362869..90d1bf6f 100644 --- a/app/client/models/UserManagerModel.ts +++ b/app/client/models/UserManagerModel.ts @@ -32,6 +32,8 @@ export interface UserManagerModel { activeUser: FullUser|null; // Populated if current user is logged in. gristDoc: GristDoc|null; // Populated if there is an open document. + // Analyze the relation that users have to the resource or site. + annotate(): void; // Resets all unsaved changes reset(): void; // Recreate annotations, factoring in any changes on the back-end. @@ -253,7 +255,6 @@ export class UserManagerModelImpl extends Disposable implements UserManagerModel return member.email === this.activeUser?.email; } - // Analyze the relation that users have to the resource or site. public annotate() { // Only attempt for documents for now. // TODO: extend to workspaces. diff --git a/app/client/ui/UserManager.ts b/app/client/ui/UserManager.ts index 3773343c..beb7c2fa 100644 --- a/app/client/ui/UserManager.ts +++ b/app/client/ui/UserManager.ts @@ -10,7 +10,7 @@ import {isLongerThan} from 'app/common/gutil'; import {FullUser} from 'app/common/LoginSessionAPI'; import * as roles from 'app/common/roles'; import {Organization, PermissionData, UserAPI} from 'app/common/UserAPI'; -import {Computed, Disposable, keyframes, observable, Observable, dom, DomElementArg, styled} from 'grainjs'; +import {Computed, Disposable, dom, DomElementArg, keyframes, Observable, observable, styled} from 'grainjs'; import pick = require('lodash/pick'); import {ACIndexImpl, normalizeText} from 'app/client/lib/ACIndex'; @@ -204,9 +204,8 @@ export class UserManager extends Disposable { } public buildDom() { - const acmemberEmail = this.autoDispose(new ACMemberEmail( + const acMemberEmail = this.autoDispose(new ACMemberEmail( this._onAdd.bind(this), - (member) => this._model.isActiveUser(member), this._model.membersEdited.get(), this._options.prompt, )); @@ -219,7 +218,7 @@ export class UserManager extends Disposable { } return [ - acmemberEmail.buildDom(), + acMemberEmail.buildDom(), this._buildOptionsDom(), this._dom = shadowScroll( testId('um-members'), @@ -531,7 +530,7 @@ function getUserItem(member: IEditableMember): ACUserItem { name: member.name, picture: member?.picture, id: member.id, - } + }; } /** @@ -539,11 +538,9 @@ function getUserItem(member: IEditableMember): ACUserItem { */ export class ACMemberEmail extends Disposable { private _email = this.autoDispose(observable("")); - private _isValid = this.autoDispose(observable(false)); constructor( private _onAdd: (email: string, role: roles.NonGuestRole) => void, - private _isActiveUser: (member: IEditableMember) => boolean, private _members: Array, private _prompt?: {email: string} ) { @@ -562,7 +559,6 @@ export class ACMemberEmail extends Disposable { acIndex, emailObs: this._email, save: this._handleSave.bind(this), - isInputValid: this._isValid, prompt: this._prompt, }, testId('um-member-new') @@ -570,12 +566,7 @@ export class ACMemberEmail extends Disposable { } private _handleSave(selectedEmail: string) { - const member = this._members.find(member => member.email === selectedEmail); - if (!member) { - this._onAdd(selectedEmail, roles.VIEWER); - } else if (!this._isActiveUser(member)) { - member?.effectiveAccess.set(roles.VIEWER); - } + this._onAdd(selectedEmail, roles.VIEWER); } }