Skip to content

Commit

Permalink
(core) Revert "Document type conversion UX/UI (#1181)"
Browse files Browse the repository at this point in the history 
Summary:
The recently-landed document type conversion feature was broken, failing
to change the document's type in both Jenkins CI runs and during manual
testing of the SaaS build of Grist.

This reverts the feature until a fix is ready.

Test Plan: N/A

Reviewers: dsagal

Reviewed By: dsagal

Differential Revision: https://phab.getgrist.com/D4411
  • Loading branch information
@georgegevoian
georgegevoian committed Dec 10, 2024
1 parent 10b0690 commit 8453a02
Show file tree
Hide file tree
Showing 11 changed files with 58 additions and 514 deletions.
13 changes: 4 additions & 9 deletions app/client/models/DocPageModel.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import {Holder, Observable, subscribe} from 'grainjs';
import {Computed, Disposable, dom, DomArg, DomElementArg} from 'grainjs';
import {makeT} from 'app/client/lib/localization';
import {logTelemetryEvent} from 'app/client/lib/telemetry';
import {DocumentType} from 'app/common/UserAPI';

// tslint:disable:no-console

Expand Down Expand Up @@ -88,7 +87,7 @@ export interface DocPageModel {
isTutorialTrunk: Observable<boolean>;
isTutorialFork: Observable<boolean>;
isTemplate: Observable<boolean>;
type: Observable<DocumentType>;

importSources: ImportSource[];

undoState: Observable<IUndoState|null>; // See UndoStack for details.
Expand Down Expand Up @@ -148,8 +147,6 @@ export class DocPageModelImpl extends Disposable implements DocPageModel {
(use, doc) => doc ? doc.isTutorialFork : false);
public readonly isTemplate = Computed.create(this, this.currentDoc,
(use, doc) => doc ? doc.isTemplate : false);
public readonly type = Computed.create(this, this.currentDoc,
(use, doc) => doc?.type ?? null);

public readonly importSources: ImportSource[] = [];

Expand Down Expand Up @@ -502,8 +499,7 @@ function buildDocInfo(doc: Document, mode: OpenDocMode | undefined): DocInfo {
const isFork = Boolean(idParts.forkId || idParts.snapshotId);
const isBareFork = isFork && idParts.trunkId === NEW_DOCUMENT_CODE;
const isSnapshot = Boolean(idParts.snapshotId);
const type = doc.type;
const isTutorial = type === 'tutorial';
const isTutorial = doc.type === 'tutorial';
const isTutorialTrunk = isTutorial && !isFork && mode !== 'default';
const isTutorialFork = isTutorial && isFork;

Expand All @@ -515,7 +511,7 @@ function buildDocInfo(doc: Document, mode: OpenDocMode | undefined): DocInfo {
// mode. Since the document's 'openMode' has no effect, don't bother trying
// to set it here, as it'll potentially be confusing for other code reading it.
openMode = 'default';
} else if (!isFork && type === 'template') {
} else if (!isFork && doc.type === 'template') {
// Templates should always open in fork mode by default.
openMode = 'fork';
} else {
Expand All @@ -525,7 +521,7 @@ function buildDocInfo(doc: Document, mode: OpenDocMode | undefined): DocInfo {
}

const isPreFork = openMode === 'fork';
const isTemplate = type === 'template' && (isFork || isPreFork);
const isTemplate = doc.type === 'template' && (isFork || isPreFork);
const isEditable = !isSnapshot && (canEdit(doc.access) || isPreFork);
return {
...doc,
Expand All @@ -538,7 +534,6 @@ function buildDocInfo(doc: Document, mode: OpenDocMode | undefined): DocInfo {
isSnapshot,
isTutorialTrunk,
isTutorialFork,
type,
isTemplate,
isReadonly: !isEditable,
idParts,
Expand Down
201 changes: 11 additions & 190 deletions app/client/ui/DocumentSettings.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,18 +29,7 @@ import {commonUrls, GristLoadConfig} from 'app/common/gristUrls';
import {not, propertyCompare} from 'app/common/gutil';
import {getCurrency, locales} from 'app/common/Locales';
import {isOwner, isOwnerOrEditor} from 'app/common/roles';
import {DOCTYPE_NORMAL, DOCTYPE_TEMPLATE, DOCTYPE_TUTORIAL, DocumentType, persistType} from 'app/common/UserAPI';
import {
Computed,
Disposable,
dom,
DomElementMethod,
fromKo,
IDisposableOwner,
makeTestId,
Observable,
styled
} from 'grainjs';
import {Computed, Disposable, dom, fromKo, IDisposableOwner, makeTestId, Observable, styled} from 'grainjs';
import * as moment from 'moment-timezone';

const t = makeT('DocumentSettings');
Expand Down Expand Up @@ -96,22 +85,6 @@ export class DocSettingsPage extends Disposable {
{defaultCurrencyLabel: t("Local currency ({{currency}})", {currency: getCurrency(l)})})
)
}),
dom.create(AdminSectionItem, {
id: 'templateMode',
name: t('Template mode'),
description: t('Change document type'),
value: cssDocTypeContainer(
dom.create(
displayCurrentType,
docPageModel.type,
),
cssSmallButton(t('Edit'),
dom.on('click', this._buildDocumentTypeModal.bind(this, true)),
testId('doctype-edit')
),
),
disabled: isDocOwner ? false : t('Only available to document owners'),
}),
]),

dom.create(AdminSection, t('Data Engine'), [
Expand Down Expand Up @@ -147,13 +120,15 @@ export class DocSettingsPage extends Disposable {
)),
disabled: isDocOwner ? false : t('Only available to document owners'),
}),

dom.create(AdminSectionItem, {
id: 'reload',
name: t('Reload'),
description: t('Hard reset of data engine'),
value: cssSmallButton(t('Reload data engine'), dom.on('click', this._reloadEngine.bind(this, true))),
disabled: isDocEditor ? false : t('Only available to document editors'),
}),

canChangeEngine ? dom.create(AdminSectionItem, {
id: 'python',
name: t('Python'),
Expand Down Expand Up @@ -211,6 +186,7 @@ export class DocSettingsPage extends Disposable {
href: getApiConsoleLink(docPageModel),
}),
}),

dom.create(AdminSectionItem, {
id: 'webhooks',
name: t('Webhooks'),
Expand Down Expand Up @@ -248,11 +224,11 @@ export class DocSettingsPage extends Disposable {
const docPageModel = this._gristDoc.docPageModel;
modal((ctl, owner) => {
this.onDispose(() => ctl.close());
const selected = Observable.create<TimingModalOption>(owner, TimingModalOption.Adhoc);
const selected = Observable.create<Option>(owner, Option.Adhoc);
const page = Observable.create<TimingModalPage>(owner, TimingModalPage.Start);

const startTiming = async () => {
if (selected.get() === TimingModalOption.Reload) {
if (selected.get() === Option.Reload) {
page.set(TimingModalPage.Spinner);
await this._gristDoc.docApi.startTiming();
await docPageModel.appModel.api.getDocAPI(docPageModel.currentDocId.get()!).forceReload();
Expand All @@ -267,7 +243,7 @@ export class DocSettingsPage extends Disposable {
const startPage = () => [
cssRadioCheckboxOptions(
dom.style('max-width', '400px'),
radioCheckboxOption(selected, TimingModalOption.Adhoc, dom('div',
radioCheckboxOption(selected, Option.Adhoc, dom('div',
dom('div',
dom('strong', t('Start timing')),
),
Expand All @@ -277,7 +253,7 @@ export class DocSettingsPage extends Disposable {
),
testId('timing-modal-option-adhoc'),
)),
radioCheckboxOption(selected, TimingModalOption.Reload, dom('div',
radioCheckboxOption(selected, Option.Reload, dom('div',
dom('div',
dom('strong', t('Time reload')),
),
Expand Down Expand Up @@ -313,111 +289,6 @@ export class DocSettingsPage extends Disposable {
});
}

private _buildDocumentTypeModal() {
const docPageModel = this._gristDoc.docPageModel;
modal((ctl, owner) => {
this.onDispose(() => ctl.close());
const currentDocType = docPageModel.type.get() as string;
let currentDocTypeOption;
switch (currentDocType) {
case DOCTYPE_TEMPLATE:
currentDocTypeOption = DocTypeOption.Template;
break;
case DOCTYPE_TUTORIAL:
currentDocTypeOption = DocTypeOption.Tutorial;
break;
default:
currentDocTypeOption = DocTypeOption.Regular;
}

const selected = Observable.create<DocTypeOption>(owner, currentDocTypeOption);

const doSetDocumentType = async () => {
const docId = docPageModel.currentDocId.get();
let docType: DocumentType;
if (selected.get() === DocTypeOption.Regular) {
docType = DOCTYPE_NORMAL;
} else if (selected.get() === DocTypeOption.Template) {
docType = DOCTYPE_TEMPLATE;
} else {
docType = DOCTYPE_TUTORIAL;
}
await persistType(docType, docId);
const {trunkId} = docPageModel.currentDoc.get()!.idParts;
window.location.replace(urlState().makeUrl({
docPage: "settings",
fork: undefined, // will be automatically set once the page is reloaded
doc: trunkId,
}));
};

const docTypeOption = (
{
type,
label,
description,
itemTestId
}: {
type: DocTypeOption,
label: string | any,
description: string,
itemTestId: DomElementMethod | null
}) => {
return radioCheckboxOption(selected, type, dom('div',
dom('div',
dom('strong', label),
),
dom('div',
dom.style('margin-top', '8px'),
dom('span', description)
),
itemTestId,
));
};

const documentTypeOptions = () => [
cssRadioCheckboxOptions(
dom.style('max-width', '400px'),
docTypeOption({
type: DocTypeOption.Regular,
label: t('Regular document'),
description: t('Normal document behavior. All users work on the same copy of the document.'),
itemTestId: testId('doctype-modal-option-regular'),
}),
docTypeOption({
type: DocTypeOption.Template,
label: t('Template'),
description: t('Document automatically opens in {{fiddleModeDocUrl}}. ' +
'Anyone may edit, which will create a new unsaved copy.',
{
fiddleModeDocUrl: cssLink({href: commonUrls.helpAPI, target: '_blank'}, t('fiddle mode'))
}
),
itemTestId: testId('doctype-modal-option-template'),
}),
docTypeOption({
type: DocTypeOption.Tutorial,
label: t('Tutorial'),
description: t('Document automatically opens as a user-specific copy.'),
itemTestId: testId('doctype-modal-option-tutorial'),
}),
),
cssModalButtons(
bigBasicButton(t('Cancel'), dom.on('click', () => ctl.close()), testId('doctype-modal-cancel')),
bigPrimaryButton(t(`Confirm change`),
dom.on('click', doSetDocumentType),
testId('doctype-modal-confirm'),
),
)
];
return [
cssModalTitle(t(`Change document type`)),
documentTypeOptions(),
testId('doctype-modal'),
];
});
}

private async _doSetEngine(val: EngineCode|undefined) {
const docPageModel = this._gristDoc.docPageModel;
if (this._engine.get() !== val) {
Expand All @@ -427,6 +298,8 @@ export class DocSettingsPage extends Disposable {
}
}



function getApiConsoleLink(docPageModel: DocPageModel) {
const url = new URL(location.href);
url.pathname = '/apiconsole';
Expand Down Expand Up @@ -470,39 +343,6 @@ function buildLocaleSelect(
);
}

type DocumentTypeItem = ACSelectItem & {type?: string};

const typeList: DocumentTypeItem[] = [{
label: t('Regular'),
type: ''
}, {
label: t('Template'),
type: 'template'
}, {
label: t('Tutorial'),
type: 'tutorial'
}].map((el) => ({
...el,
value: el.label,
cleanText: el.label.trim().toLowerCase()
}));

function displayCurrentType(
owner: IDisposableOwner,
type: Observable<DocumentType|null>,
) {
const typeObs = Computed.create(owner, use => {
const typeCode = use(type) ?? "";
const typeName = typeList.find(ty => ty.type === typeCode)?.label || typeCode;
return typeName;
});
return dom(
'div',
typeObs.get(),
testId('doctype-value')
);
}

const cssContainer = styled('div', `
overflow-y: auto;
position: relative;
Expand Down Expand Up @@ -622,7 +462,7 @@ enum TimingModalPage {
/**
* Enum for the different options in the timing modal.
*/
enum TimingModalOption {
enum Option {
/**
* Start timing and immediately forces a reload of the document and waits for the
* document to be loaded, to show the results.
Expand All @@ -634,15 +474,6 @@ enum TimingModalOption {
Adhoc,
}

/**
* Enum for the different options in the document type Modal.
*/
enum DocTypeOption {
Regular,
Template,
Tutorial,
}

// A version that is not underlined, and on hover mouse pointer indicates that copy is available
const cssCopyLink = styled(cssLink, `
word-wrap: break-word;
Expand Down Expand Up @@ -678,13 +509,3 @@ const cssWrap = styled('p', `
const cssRedText = styled('span', `
color: ${theme.errorText};
`);

const cssDocTypeContainer = styled('div', `
display: flex;
width: 172px;
align-items: center;
justify-content: space-between;
& > * {
display: inline-block;
}
`);
1 change: 0 additions & 1 deletion app/client/ui2018/cssVars.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,6 @@ export const theme = {

/* Checkboxes */
checkboxBg: new CustomProp('theme-checkbox-bg', undefined, colors.light),
checkboxSelectedFg: new CustomProp('theme-checkbox-selected-bg', undefined, colors.lightGreen),
checkboxDisabledBg: new CustomProp('theme-checkbox-disabled-bg', undefined, colors.darkGrey),
checkboxBorder: new CustomProp('theme-checkbox-border', undefined, colors.darkGrey),
checkboxBorderHover: new CustomProp('theme-checkbox-border-hover', undefined, colors.hover),
Expand Down
17 changes: 1 addition & 16 deletions app/common/UserAPI.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,22 +128,7 @@ export interface Workspace extends WorkspaceProperties {
isSupportWorkspace?: boolean;
}

export const DOCTYPE_NORMAL = null;
export const DOCTYPE_TEMPLATE = 'template';
export const DOCTYPE_TUTORIAL = 'tutorial';

// null stands for normal document type, the one set by default at document creation.
export type DocumentType = 'template'|'tutorial'|null;

export function persistType(type: DocumentType, docId: string|undefined) {
docId = docId?.split("~")[0];
return fetch(`/o/docs/api/docs/${docId}`, {
method: 'PATCH',
headers: {"Content-Type": "application/json"},
credentials: 'include',
body: JSON.stringify({type})
});
}
export type DocumentType = 'tutorial'|'template';

// Non-core options for a document.
// "Non-core" means bundled into a single options column in the database.
Expand Down
Loading

0 comments on commit 8453a02

Please sign in to comment.