Skip to content

Commit

Permalink
#845 Don't generate subjects with special characters
Browse files Browse the repository at this point in the history
  • Loading branch information
Polleps committed Apr 18, 2024
1 parent cad74e8 commit 2223aba
Show file tree
Hide file tree
Showing 16 changed files with 175 additions and 164 deletions.
24 changes: 13 additions & 11 deletions browser/data-browser/src/components/SideBar/DriveSwitcher.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -48,17 +48,19 @@ export function DriveSwitcher() {

const items = useMemo(
() => [
...Array.from(savedDrivesMap.entries()).map(([subject, resource]) => ({
id: subject,
label: getTitle(resource),
helper: `Switch to ${getTitle(resource)}`,
disabled: subject === drive,
onClick: () => {
setDrive(subject);
navigate(constructOpenURL(subject));
},
icon: subject === drive ? <FaRegCheckCircle /> : <FaRegCircle />,
})),
...Array.from(savedDrivesMap.entries())
.filter(([_, resource]) => !resource.error)
.map(([subject, resource]) => ({
id: subject,
label: getTitle(resource),
helper: `Switch to ${getTitle(resource)}`,
disabled: subject === drive,
onClick: () => {
setDrive(subject);
navigate(constructOpenURL(subject));
},
icon: subject === drive ? <FaRegCheckCircle /> : <FaRegCircle />,
})),
DIVIDER,
// Dedupe history from savedDrives bause not all savedDrives might be loaded yet.
...Array.from(dedupeAFromB(historyMap, savedDrivesMap))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export const registerBasicInstanceHandlers = () => {
await createAndNavigate(
dataBrowser.classes.folder,
{
[core.properties.name]: 'untitled-folder',
[core.properties.name]: 'Untitled Folder',
[dataBrowser.properties.displayStyle]: classes.displayStyles.list,
},
parent,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { core, useStore, server, dataBrowser } from '@tomic/react';
import { useState, useCallback, FormEvent, FC, useEffect } from 'react';
import { useState, useCallback, FormEvent, FC, useEffect, useId } from 'react';
import { styled } from 'styled-components';
import { stringToSlug } from '../../../../../helpers/stringToSlug';
import { Button } from '../../../../Button';
Expand All @@ -20,6 +20,7 @@ export const NewDriveDialog: FC<CustomResourceDialogProps> = ({
onClose,
}) => {
const store = useStore();
const nameFieldId = useId();
const { setDrive } = useSettings();
const [name, setName] = useState('');

Expand Down Expand Up @@ -105,9 +106,10 @@ export const NewDriveDialog: FC<CustomResourceDialogProps> = ({
hide(true);
}}
>
<Field required label='Name'>
<Field required label='Name' fieldId={nameFieldId}>
<InputWrapper>
<InputStyled
id={nameFieldId}
placeholder='My Drive'
value={name}
autoFocus={true}
Expand Down
10 changes: 10 additions & 0 deletions browser/data-browser/src/helpers/useOnValueChange.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { useState } from 'react';

export function useOnValueChange(callback: () => void, dependants: unknown[]) {
const [deps, setDeps] = useState(dependants);

if (deps.some((d, i) => d !== dependants[i])) {
setDeps(dependants);
callback();
}
}
1 change: 1 addition & 0 deletions browser/data-browser/src/views/DrivePage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ function DrivePage({ resource }: ResourcePageProps): JSX.Element {
<div>
<Heading>Default Ontology</Heading>
<InputSwitcher
commit
resource={resource}
property={defaultOntologyProp}
disabled={!canEdit}
Expand Down
39 changes: 33 additions & 6 deletions browser/data-browser/src/views/ResourceInline/ResourceInline.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,14 @@
import { useString, useResource, urls, Client, useArray } from '@tomic/react';
import {
useString,
useResource,
Client,
useArray,
isAtomicError,
ErrorType,
core,
dataBrowser,
server,
} from '@tomic/react';
import { AtomicLink } from '../../components/AtomicLink';
import { ErrorLook } from '../../components/ErrorLook';
import { LoaderInline } from '../../components/Loader';
Expand All @@ -15,6 +25,23 @@ type ResourceInlineProps = {
basic?: boolean;
} & ResourceInlineInstanceProps;

function getMessageForErrorType(error: Error) {
if (isAtomicError(error)) {
switch (error.type) {
case ErrorType.NotFound:
return 'Resource not found';
case ErrorType.Unauthorized:
return 'Unauthorized';
case ErrorType.Server:
return 'Server error';
case ErrorType.Client:
return 'Something went wrong';
}
} else {
return 'Error loading resource';
}
}

/** Renders a Resource in a compact, inline link. Shows tooltip on hover. */
export function ResourceInline({
subject,
Expand All @@ -23,7 +50,7 @@ export function ResourceInline({
className,
}: ResourceInlineProps): JSX.Element {
const resource = useResource(subject, { allowIncomplete: true });
const [isA] = useArray(resource, urls.properties.isA);
const [isA] = useArray(resource, core.properties.isA);

const Comp = basic ? DefaultInline : classMap.get(isA[0]) ?? DefaultInline;

Expand All @@ -35,7 +62,7 @@ export function ResourceInline({
return (
<AtomicLink subject={subject} untabbable={untabbable}>
<ErrorLook about={subject} title={resource.error.message}>
Unknown Resource
{getMessageForErrorType(resource.error)}
</ErrorLook>
</AtomicLink>
);
Expand All @@ -58,7 +85,7 @@ export function ResourceInline({

function DefaultInline({ subject }: ResourceInlineInstanceProps): JSX.Element {
const resource = useResource(subject);
const [description] = useString(resource, urls.properties.description);
const [description] = useString(resource, core.properties.description);

return <span title={description ? description : ''}>{resource.title}</span>;
}
Expand All @@ -67,6 +94,6 @@ const classMap = new Map<
string,
(props: ResourceInlineInstanceProps) => JSX.Element
>([
[urls.classes.tag, TagInline],
[urls.classes.file, FileInline],
[dataBrowser.classes.tag, TagInline],
[server.classes.file, FileInline],
]);
7 changes: 5 additions & 2 deletions browser/data-browser/src/views/TablePage/TableCell.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,12 @@ export function TableCell({
const handleEditNextRow = useCallback(() => {
if (!savePending) {
onEditNextRow?.();
setTimeout(() => {

// Only go to the next row if the resource has any properties set (It has two by default, isA and parent)
// This prevents triggering a rerender and losing focus on the input.
if (resource.getPropVals().size > 2) {
setActiveCell(rowIndex + 1, columnIndex);
}, 0);
}
}
}, [savePending, setActiveCell, rowIndex, columnIndex]);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,14 @@ import {
CursorMode,
useTableEditorContext,
} from '../../components/TableEditor/TableEditorContext';
import { useOnValueChange } from '../../helpers/useOnValueChange';

export function useTableInvalidation(
resource: Resource,
invalidateTable: () => void,
) {
const store = useStore();
const { cursorMode } = useTableEditorContext();
const { cursorMode, selectedColumn, selectedRow } = useTableEditorContext();

const [markedForInvalidation, setMarkedForInvalidation] = useState(false);

Expand All @@ -20,6 +21,12 @@ export function useTableInvalidation(
}
}, [invalidateTable, markedForInvalidation]);

useOnValueChange(() => {
if (markedForInvalidation) {
invalidateTable();
}
}, [selectedRow, selectedColumn]);

useEffect(() => {
if (markedForInvalidation && cursorMode !== CursorMode.Edit) {
invalidateTable();
Expand Down
2 changes: 1 addition & 1 deletion browser/e2e/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
"url": "https://github.com/atomicdata-dev/atomic-server/"
},
"devDependencies": {
"@playwright/test": "^1.37.0"
"@playwright/test": "^1.43.1"
},
"scripts": {
"playwright-install": "playwright install chromium",
Expand Down
5 changes: 3 additions & 2 deletions browser/e2e/tests/tables.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,11 @@ test.describe('tables', async () => {
await page.keyboard.press('Enter');
await expect(
page.locator(
`[aria-rowindex="${rowIndex}"] > [aria-colindex="${2}"] > input`,
`[aria-rowindex="${rowIndex}"] > [aria-colindex="2"] > input`,
),
).toBeFocused();
await page.keyboard.type(name);
await page.waitForTimeout(300);
await tab();
// Flay newline
await page.waitForTimeout(300);
Expand Down Expand Up @@ -86,7 +87,7 @@ test.describe('tables', async () => {
).toBeVisible();
await expect(
page.locator(
`[aria-rowindex="${rowIndex + 1}"] > [aria-colindex="${2}"] > input`,
`[aria-rowindex="${rowIndex + 1}"] > [aria-colindex="2"] > input`,
),
"Next row's first cell isn't focused",
).toBeFocused();
Expand Down
10 changes: 8 additions & 2 deletions browser/e2e/tests/test-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,14 +101,20 @@ export async function signIn(page: Page) {
*/
export async function newDrive(page: Page) {
// Create new drive to prevent polluting the main drive
const driveTitle = `testdrive-${timestamp()}`;
await page.locator(sideBarDriveSwitcher).click();
await page.locator('button:has-text("New Drive")').click();
await expect(
currentDialog(page).getByRole('heading', { name: 'New Drive' }),
).toBeVisible();

await currentDialog(page).getByLabel('Name').fill(driveTitle);

await currentDialog(page).getByRole('button', { name: 'Create' }).click();
expect(page.locator(`${currentDriveTitle} > localhost`)).not.toBeVisible();
await expect(page.locator('text="Create new resource"')).toBeVisible();
const driveURL = await getCurrentSubject(page);
expect(driveURL).toContain('localhost');
const driveTitle = `testdrive-${timestamp()}`;
await editTitle(driveTitle, page);

return { driveURL: driveURL as string, driveTitle };
}
Expand Down
4 changes: 4 additions & 0 deletions browser/lib/src/error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ export function isUnauthorized(error?: Error): boolean {
return false;
}

export function isAtomicError(error: Error): error is AtomicError {
return error instanceof AtomicError;
}

/**
* Atomic Data Errors have an additional Type, which tells the client what kind
* of error to render.
Expand Down
11 changes: 0 additions & 11 deletions browser/lib/src/resource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -726,17 +726,6 @@ export class Resource<C extends OptionalClass = any> {
this._subject = subject;
}

/** Returns true if the value has not changed */
private equalsCurrentValue(prop: string, value: JSONValue) {
const ownValue = this.get(prop);

if (value === Object(value)) {
return JSON.stringify(ownValue) === JSON.stringify(value);
}

return ownValue === value;
}

private isParentNew() {
const parentSubject = this.propvals.get(core.properties.parent) as string;

Expand Down
6 changes: 3 additions & 3 deletions browser/lib/src/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
buildSearchSubject,
server,
} from './index.js';
import { stringToSlug } from './stringToSlug.js';
import { authenticate, fetchWebSocket, startWebsocket } from './websockets.js';

/** Function called when a resource is updated or removed */
Expand Down Expand Up @@ -288,7 +289,7 @@ export class Store {
parts: string[],
parent?: string,
): Promise<string> {
const path = parts.join('/');
const path = parts.map(part => stringToSlug(part)).join('/');
const parentUrl = parent ?? this.getServerUrl();

return this.findAvailableSubject(path, parentUrl);
Expand Down Expand Up @@ -941,7 +942,7 @@ export class Store {
parent: string,
firstTry = true,
): Promise<string> {
let url = `${parent}/${path}`;
let url = new URL(`${parent}/${path}`).toString();

if (!firstTry) {
const randomPart = this.randomPart();
Expand All @@ -968,7 +969,6 @@ export class Store {
return;
}

// We clone for react, because otherwise it won't rerender
Promise.allSettled(callbacks.map(async cb => cb(resource)));
}
}
Expand Down
7 changes: 7 additions & 0 deletions browser/lib/src/stringToSlug.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export function stringToSlug(str: string): string {
return str
.toLowerCase()
.replace(/\s+/g, '-')
.replace(/-+/g, '-')
.replace(/[^\w-]+/g, '');
}
Loading

0 comments on commit 2223aba

Please sign in to comment.