From b7f65ff408bf0483eaaa0a28255badc89d3a2eea Mon Sep 17 00:00:00 2001 From: Cyprien P Date: Mon, 9 Jan 2023 17:26:09 +0100 Subject: [PATCH] (core) Adds dots menu to access rules page item MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: In Access rules page item, adds “…” buttons that shows a menu of users to view-as: Test Plan: Include new nbrowser test Reviewers: jarek Reviewed By: jarek Differential Revision: https://phab.getgrist.com/D3751 --- app/client/aclui/ACLUsers.ts | 74 +++++++++++++++++++++++---- app/client/aclui/AccessRules.ts | 12 ++--- app/client/components/ViewAsBanner.ts | 5 +- app/client/ui/HomeLeftPane.ts | 24 ++------- app/client/ui/LeftPanelCommon.ts | 20 ++++++++ app/client/ui/Tools.ts | 26 +++++++++- app/common/getCurrentTime.ts | 2 +- test/nbrowser/gristUtils.ts | 8 +++ 8 files changed, 125 insertions(+), 46 deletions(-) diff --git a/app/client/aclui/ACLUsers.ts b/app/client/aclui/ACLUsers.ts index 1c6ef2c0..4cdff1a2 100644 --- a/app/client/aclui/ACLUsers.ts +++ b/app/client/aclui/ACLUsers.ts @@ -4,18 +4,20 @@ import {createUserImage} from 'app/client/ui/UserImage'; import {cssMemberImage, cssMemberListItem, cssMemberPrimary, cssMemberSecondary, cssMemberText} from 'app/client/ui/UserItem'; import {testId, theme, vars} from 'app/client/ui2018/cssVars'; -import {menuCssClass} from 'app/client/ui2018/menus'; import {PermissionDataWithExtraUsers} from 'app/common/ActiveDocAPI'; +import {menu, menuCssClass, menuItemLink} from 'app/client/ui2018/menus'; import {userOverrideParams} from 'app/common/gristUrls'; import {FullUser} from 'app/common/LoginSessionAPI'; import {ANONYMOUS_USER_EMAIL, EVERYONE_EMAIL} from 'app/common/UserAPI'; import {getRealAccess, UserAccessData} from 'app/common/UserAPI'; import {Disposable, dom, Observable, styled} from 'grainjs'; -import {cssMenu, cssMenuWrap, defaultMenuOptions, IPopupOptions, setPopupToCreateDom} from 'popweasel'; +import {cssMenu, cssMenuWrap, defaultMenuOptions, IMenuOptions, IPopupOptions, setPopupToCreateDom} from 'popweasel'; import {getUserRoleText} from 'app/common/UserAPI'; import {makeT} from 'app/client/lib/localization'; +import {waitGrainObs} from 'app/common/gutil'; +import noop from 'lodash/noop'; -const t = makeT("ACLUsers"); +const t = makeT("ViewAsDropdown"); function isSpecialEmail(email: string) { return email === ANONYMOUS_USER_EMAIL || email === EVERYONE_EMAIL; @@ -23,15 +25,33 @@ function isSpecialEmail(email: string) { export class ACLUsersPopup extends Disposable { public readonly isInitialized = Observable.create(this, false); + public readonly allUsers = Observable.create(this, []); private _shareUsers: UserAccessData[] = []; // Users doc is shared with. private _attributeTableUsers: UserAccessData[] = []; // Users mentioned in attribute tables. private _exampleUsers: UserAccessData[] = []; // Example users. private _currentUser: FullUser|null = null; - private _pageModel: DocPageModel|null = null; - public init(pageModel: DocPageModel, permissionData: PermissionDataWithExtraUsers|null) { + constructor(public pageModel: DocPageModel, + public fetch: () => Promise = () => this._fetchData()) { + super(); + } + + public async load() { + const permissionData = await this.fetch(); + if (this.isDisposed()) { return; } + this.init(permissionData); + } + + public getUsers() { + const users = [...this._shareUsers, ...this._attributeTableUsers]; + if (this._showExampleUsers()) { users.push(...this._exampleUsers); } + return users; + } + + public init(permissionData: PermissionDataWithExtraUsers|null) { + const pageModel = this.pageModel; this._currentUser = pageModel.userOverride.get()?.user || pageModel.appModel.currentValidUser; - this._pageModel = pageModel; + if (permissionData) { this._shareUsers = permissionData.users.map(user => ({ ...user, @@ -41,6 +61,7 @@ export class ACLUsersPopup extends Disposable { .filter(user => this._currentUser?.id !== user.id); this._attributeTableUsers = permissionData.attributeTableUsers; this._exampleUsers = permissionData.exampleUsers; + this.allUsers.set(this.getUsers()); this.isInitialized.set(true); } } @@ -61,7 +82,7 @@ export class ACLUsersPopup extends Disposable { // Include example users only if there are not many "real" users. // It might be better to have an expandable section with these users, collapsed // by default, but that's beyond my UI ken. - (this._shareUsers.length + this._attributeTableUsers.length < 5) ? [ + this._showExampleUsers() ? [ (this._exampleUsers.length > 0) ? cssHeader(t("Example Users")) : null, dom.forEach(this._exampleUsers, buildExampleUserRow) ] : null, @@ -71,6 +92,30 @@ export class ACLUsersPopup extends Disposable { }, {...defaultMenuOptions, ...options}); } + public menu(options: IMenuOptions) { + return menu(() => { + this.load().catch(noop); + return [ + cssMenuHeader('view as'), + dom.forEach(this.allUsers, user => menuItemLink( + `${user.name || user.email} (${getUserRoleText(user)})`, + testId('acl-user-access'), + this._viewAs(user), + )), + ]; + }, options); + } + + private async _fetchData() { + const doc = this.pageModel.currentDoc.get(); + const gristDoc = await waitGrainObs(this.pageModel.gristDoc); + return doc && gristDoc.docComm.getUsersForViewAs(); + } + + private _showExampleUsers() { + return this._shareUsers.length + this._attributeTableUsers.length < 5; + } + private _buildUserRow(user: UserAccessData, opt: {isExampleUser?: boolean} = {}) { return dom('a', {class: cssMemberListItem.className + ' ' + cssUserItem.className}, @@ -89,15 +134,15 @@ export class ACLUsersPopup extends Disposable { } private _viewAs(user: UserAccessData) { - if (this._pageModel?.isPrefork.get() && - this._pageModel?.currentDoc.get()?.access !== 'owners') { + if (this.pageModel?.isPrefork.get() && + this.pageModel?.currentDoc.get()?.access !== 'owners') { // "View As" is restricted to document owners on the back-end. Non-owners can be // permitted to pretend to be owners of a pre-forked document, but if they want // to do "View As", that would be layering pretence over pretense. Better to just // go ahead and create the fork, so the user becomes a genuine owner, so the // back-end doesn't have to become too metaphysical (and maybe hard to review). return dom.on('click', async () => { - const forkResult = await this._pageModel?.gristDoc.get()?.docComm.fork(); + const forkResult = await this.pageModel?.gristDoc.get()?.docComm.fork(); if (!forkResult) { throw new Error('Failed to create fork'); } window.location.assign(urlState().makeUrl(userOverrideParams(user.email, {doc: forkResult.urlId, @@ -139,3 +184,12 @@ const cssHeader = styled('div', ` font-size: ${vars.xsmallFontSize}; color: ${theme.darkText}; `); + +const cssMenuHeader = styled('div', ` + margin: 8px 24px; + margin-bottom: 4px; + font-weight: 700; + text-transform: uppercase; + font-size: ${vars.xsmallFontSize}; + color: ${theme.darkText}; +`); diff --git a/app/client/aclui/AccessRules.ts b/app/client/aclui/AccessRules.ts index cf055050..8b1c1fd7 100644 --- a/app/client/aclui/AccessRules.ts +++ b/app/client/aclui/AccessRules.ts @@ -122,7 +122,7 @@ export class AccessRules extends Disposable { // Map of tableId to basic metadata for all tables in the document. private _aclResources = new Map(); - private _aclUsersPopup = ACLUsersPopup.create(this); + private _aclUsersPopup = ACLUsersPopup.create(this, this._gristDoc.docPageModel); constructor(private _gristDoc: GristDoc) { super(); @@ -531,13 +531,7 @@ export class AccessRules extends Disposable { } private async _updateDocAccessData() { - const pageModel = this._gristDoc.docPageModel; - const doc = pageModel.currentDoc.get(); - - const permissionData = doc && await this._gristDoc.docComm.getUsersForViewAs(); - if (this.isDisposed()) { return; } - - this._aclUsersPopup.init(pageModel, permissionData); + await this._aclUsersPopup.load(); } private _addButtonsForMissingTables(buttons: Array, tableIds: string[]) { @@ -625,7 +619,7 @@ class TableRules extends Disposable { } else if (this._defRuleSet) { DefaultObsRuleSet.create(this._defaultRuleSet, this._accessRules, this, this._haveColumnRules, this._defRuleSet); - } + } this.ruleStatus = Computed.create(this, (use) => { const columnRuleSets = use(this._columnRuleSets); diff --git a/app/client/components/ViewAsBanner.ts b/app/client/components/ViewAsBanner.ts index c42aecee..b02bc087 100644 --- a/app/client/components/ViewAsBanner.ts +++ b/app/client/components/ViewAsBanner.ts @@ -21,7 +21,7 @@ const t = makeT('components.ViewAsBanner'); export class ViewAsBanner extends Disposable { private _userOverride = this._docPageModel.userOverride; - private _usersPopup = ACLUsersPopup.create(this); + private _usersPopup = ACLUsersPopup.create(this, this._docPageModel, this._getUsersForViewAs.bind(this)); constructor (private _docPageModel: DocPageModel) { super(); @@ -73,8 +73,7 @@ export class ViewAsBanner extends Disposable { private async _initViewAsUsers() { await waitGrainObs(this._docPageModel.gristDoc); - const permissionData = await this._getUsersForViewAs(); - this._usersPopup.init(this._docPageModel, permissionData); + await this._usersPopup.load(); } private _getUsersForViewAs(): Promise { diff --git a/app/client/ui/HomeLeftPane.ts b/app/client/ui/HomeLeftPane.ts index edc90c9e..069c1357 100644 --- a/app/client/ui/HomeLeftPane.ts +++ b/app/client/ui/HomeLeftPane.ts @@ -7,7 +7,9 @@ import {HomeModel} from 'app/client/models/HomeModel'; import {getWorkspaceInfo, workspaceName} from 'app/client/models/WorkspaceInfo'; import {addNewButton, cssAddNewButton} from 'app/client/ui/AddNewButton'; import {docImport, importFromPlugin} from 'app/client/ui/HomeImports'; -import {cssLinkText, cssPageEntry, cssPageIcon, cssPageLink, cssSpacer} from 'app/client/ui/LeftPanelCommon'; +import { + cssLinkText, cssMenuTrigger, cssPageEntry, cssPageIcon, cssPageLink, cssSpacer +} from 'app/client/ui/LeftPanelCommon'; import {createVideoTourToolsButton} from 'app/client/ui/OpenVideoTour'; import {transientInput} from 'app/client/ui/transientInput'; import {testId, theme} from 'app/client/ui2018/cssVars'; @@ -255,23 +257,3 @@ export const cssEditorInput = styled(transientInput, ` margin-right: 16px; font-size: inherit; `); - -const cssMenuTrigger = styled('div', ` - margin: 0 4px 0 auto; - height: 24px; - width: 24px; - padding: 4px; - line-height: 0px; - border-radius: 3px; - cursor: default; - display: none; - .${cssPageLink.className}:hover > &, &.weasel-popup-open { - display: block; - } - &:hover, &.weasel-popup-open { - background-color: ${theme.pageOptionsHoverBg}; - } - .${cssPageEntry.className}-selected &:hover, .${cssPageEntry.className}-selected &.weasel-popup-open { - background-color: ${theme.pageOptionsSelectedHoverBg}; - } -`); diff --git a/app/client/ui/LeftPanelCommon.ts b/app/client/ui/LeftPanelCommon.ts index dbb23351..802e38a3 100644 --- a/app/client/ui/LeftPanelCommon.ts +++ b/app/client/ui/LeftPanelCommon.ts @@ -185,3 +185,23 @@ export const cssPageEntrySmall = styled(cssPageEntry, ` display: none; } `); + +export const cssMenuTrigger = styled('div', ` + margin: 0 4px 0 auto; + height: 24px; + width: 24px; + padding: 4px; + line-height: 0px; + border-radius: 3px; + cursor: default; + display: none; + .${cssPageLink.className}:hover > &, &.weasel-popup-open { + display: block; + } + &:hover, &.weasel-popup-open { + background-color: ${theme.pageOptionsHoverBg}; + } + .${cssPageEntry.className}-selected &:hover, .${cssPageEntry.className}-selected &.weasel-popup-open { + background-color: ${theme.pageOptionsSelectedHoverBg}; + } +`); diff --git a/app/client/ui/Tools.ts b/app/client/ui/Tools.ts index 1ddc7878..7929e6b3 100644 --- a/app/client/ui/Tools.ts +++ b/app/client/ui/Tools.ts @@ -1,10 +1,11 @@ +import {ACLUsersPopup} from 'app/client/aclui/ACLUsers'; import {makeT} from 'app/client/lib/localization'; import {GristDoc} from 'app/client/components/GristDoc'; import {urlState} from 'app/client/models/gristUrlState'; import {getUserOrgPrefObs, markAsSeen} from 'app/client/models/UserPrefs'; import {showExampleCard} from 'app/client/ui/ExampleCard'; import {buildExamples} from 'app/client/ui/ExampleInfo'; -import {createHelpTools, cssLinkText, cssPageEntry, cssPageEntryMain, cssPageEntrySmall, +import {createHelpTools, cssLinkText, cssMenuTrigger, cssPageEntry, cssPageEntryMain, cssPageEntrySmall, cssPageIcon, cssPageLink, cssSectionHeader, cssSpacer, cssSplitPageEntry, cssTools} from 'app/client/ui/LeftPanelCommon'; import {theme} from 'app/client/ui2018/cssVars'; @@ -12,6 +13,7 @@ import {icon} from 'app/client/ui2018/icons'; import {confirmModal} from 'app/client/ui2018/modals'; import {isOwner} from 'app/common/roles'; import {Disposable, dom, makeTestId, Observable, observable, styled} from 'grainjs'; +import noop from 'lodash/noop'; const testId = makeTestId('test-tools-'); const t = makeT('Tools'); @@ -33,11 +35,31 @@ export function tools(owner: Disposable, gristDoc: GristDoc, leftPanelOpen: Obse cssPageEntry( cssPageEntry.cls('-selected', (use) => use(gristDoc.activeViewId) === 'acl'), cssPageEntry.cls('-disabled', (use) => !use(canViewAccessRules)), - dom.domComputed(canViewAccessRules, (_canViewAccessRules) => { + dom.domComputedOwned(canViewAccessRules, (computedOwner, _canViewAccessRules) => { + const aclUsers = ACLUsersPopup.create(computedOwner, docPageModel); + if (_canViewAccessRules) { + aclUsers.load() + // getUsersForViewAs() could fail for couple good reasons (access deny to anon user, + // `document not found` when anon creates a new empty document, ...), users can have more + // info by opening acl page, so let's silently fail here. + .catch(noop); + } return cssPageLink( cssPageIcon('EyeShow'), cssLinkText(t("Access Rules")), _canViewAccessRules ? urlState().setLinkUrl({docPage: 'acl'}) : null, + cssMenuTrigger( + icon('Dots'), + aclUsers.menu({ + placement: 'bottom-start', + parentSelectorToMark: '.' + cssPageEntry.className + }), + + // Clicks on the menu trigger shouldn't follow the link that it's contained in. + dom.on('click', (ev) => { ev.stopPropagation(); ev.preventDefault(); }), + testId('access-rules-trigger'), + dom.show(use => use(aclUsers.isInitialized) && _canViewAccessRules), + ), ); }), testId('access-rules'), diff --git a/app/common/getCurrentTime.ts b/app/common/getCurrentTime.ts index 8201c2d7..33449178 100644 --- a/app/common/getCurrentTime.ts +++ b/app/common/getCurrentTime.ts @@ -9,5 +9,5 @@ export default function getCurrentTime(): moment.Moment { if (typeof window === 'undefined' || !window) { return getDefault(); } const searchParams = new URLSearchParams(window.location.search); - return searchParams.has('currentTime') ? moment(searchParams.get('currentTime')) : getDefault(); + return searchParams.has('currentTime') ? moment(searchParams.get('currentTime') || undefined) : getDefault(); } diff --git a/test/nbrowser/gristUtils.ts b/test/nbrowser/gristUtils.ts index 1e6cceee..26e412e5 100644 --- a/test/nbrowser/gristUtils.ts +++ b/test/nbrowser/gristUtils.ts @@ -1667,6 +1667,14 @@ export async function openDocDropdown(docNameOrRow: string|WebElement): Promise< await docRow.find('.test-dm-doc-options,.test-dm-pinned-doc-options').mouseMove().click(); } + /** + * Open ⋮ dropdown menu for doc access rules. + */ +export async function openAccessRulesDropdown(): Promise { + await driver.find('.test-tools-access-rules').mouseMove(); + await driver.find('.test-tools-access-rules-trigger').mouseMove().click(); +} + export async function editOrgAcls(): Promise { // To prevent a common flakiness problem, wait for a potentially open modal dialog // to close before attempting to open the account menu.