Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[runtime] hide queries in the browser #1155

Merged
merged 12 commits into from
Oct 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 35 additions & 3 deletions packages/toolpad-app/src/appDom/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { ConnectionStatus, AppTheme } from '../types';
import { omit, update, updateOrCreate } from '../utils/immutability';
import { camelCase, generateUniqueString, removeDiacritics } from '../utils/strings';
import { ExactEntriesOf, Maybe } from '../utils/types';
import { filterValues } from '../utils/collections';
import { mapProperties } from '../utils/collections';

export const CURRENT_APPDOM_VERSION = 2;

Expand Down Expand Up @@ -614,6 +614,23 @@ export function setNodeProp<Node extends AppDomNode, Prop extends BindableProps<
});
}

function setNamespacedProp<
Node extends AppDomNode,
Namespace extends PropNamespaces<Node>,
Prop extends keyof Node[Namespace] & string,
>(node: Node, namespace: Namespace, prop: Prop, value: Node[Namespace][Prop] | null): Node {
if (value) {
return update(node, {
[namespace]: updateOrCreate((node as Node)[namespace], {
[prop]: value,
} as any) as Partial<Node[Namespace]>,
} as Partial<Node>);
}
return update(node, {
[namespace]: omit(node[namespace], prop) as Partial<Node[Namespace]>,
} as Partial<Node>);
}

export function setNodeNamespacedProp<
Node extends AppDomNode,
Namespace extends PropNamespaces<Node>,
Expand Down Expand Up @@ -875,16 +892,31 @@ export interface RenderTree {
version?: number;
}

const frontendNodes = new Set<string>(RENDERTREE_NODES);
function createRenderTreeNode(node: AppDomNode): RenderTreeNode | null {
if (!frontendNodes.has(node.type)) {
return null;
}

if (isQuery(node) || isMutation(node)) {
node = setNamespacedProp(node, 'attributes', 'query', null);
}

return node as RenderTreeNode;
}

/**
* We need to make sure no secrets end up in the frontend html, so let's only send the
* nodes that we need to build frontend, and that we know don't contain secrets.
* TODO: Would it make sense to create a separate datastructure that represents the render tree?
*/
export function createRenderTree(dom: AppDom): RenderTree {
const frontendNodes = new Set<string>(RENDERTREE_NODES);
return {
...dom,
nodes: filterValues(dom.nodes, (node) => frontendNodes.has(node.type)) as RenderTreeNodes,
nodes: mapProperties(dom.nodes, ([id, node]) => {
const rendernode = createRenderTreeNode(node);
return rendernode ? [id, rendernode] : null;
}),
};
}

Expand Down
51 changes: 14 additions & 37 deletions test/integration/propControls/index.spec.ts
Original file line number Diff line number Diff line change
@@ -1,25 +1,9 @@
import { test, expect, Locator } from '@playwright/test';
import { test, expect } from '@playwright/test';
import { ToolpadHome } from '../../models/ToolpadHome';
import { ToolpadEditor } from '../../models/ToolpadEditor';
import clickCenter from '../../utils/clickCenter';
import domInput from './domInput.json';

async function getPropControlInputLocator(editorModel: ToolpadEditor, inputPropName: string) {
const propControlLabelHandle = await editorModel.componentEditor
.locator(`label:has-text("${inputPropName}")`)
.elementHandle();
const propControlLabelFor = await propControlLabelHandle?.getAttribute('for');

return editorModel.componentEditor.locator(`input[id="${propControlLabelFor}"]`);
}

async function getInputElementLabelLocator(editorModel: ToolpadEditor, inputLocator: Locator) {
const inputHandle = await inputLocator.elementHandle();
const inputId = await inputHandle?.getAttribute('id');

return editorModel.appCanvas.locator(`label[for="${inputId}"]`);
}

test('can control component prop values in properties control panel', async ({
page,
browserName,
Expand All @@ -40,35 +24,28 @@ test('can control component prop values in properties control panel', async ({
const firstInputLocator = canvasInputLocator.first();
await clickCenter(page, firstInputLocator);

await editorModel.componentEditor.waitFor();
await editorModel.componentEditor.waitFor({ state: 'visible' });

const labelControlInput = editorModel.componentEditor.getByLabel('label', { exact: true });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i guess i hadn't found this selector that gets the input for a label? seems pretty useful


const labelControlInputValue = await editorModel.componentEditor
.locator(`label:text-is("label")`)
.inputValue();
const labelControlInputValue = await labelControlInput.inputValue();

expect(labelControlInputValue).toBe('textField1');

// Change component prop values directly

const TEST_VALUE_1 = 'value1';

const getValueControlInputValue = async () =>
editorModel.componentEditor.locator(`label:text-is("value")`).inputValue();

expect(await getValueControlInputValue()).not.toBe(TEST_VALUE_1);
const valueControl = editorModel.componentEditor.getByLabel('value', { exact: true });
expect(await valueControl.inputValue()).not.toBe(TEST_VALUE_1);
await firstInputLocator.fill(TEST_VALUE_1);
expect(await getValueControlInputValue()).toBe(TEST_VALUE_1);
expect(await valueControl.inputValue()).toBe(TEST_VALUE_1);

// Change component prop values through controls

const firstInputLabelLocator = await getInputElementLabelLocator(editorModel, firstInputLocator);
const TEST_VALUE_2 = 'value2';
const inputByLabel = editorModel.appCanvas.getByLabel(TEST_VALUE_2, { exact: true });
await expect(inputByLabel).toHaveCount(0);
await labelControlInput.click();
await labelControlInput.fill('');
await labelControlInput.fill(TEST_VALUE_2);

await expect(firstInputLabelLocator).not.toHaveText(TEST_VALUE_2);

const labelControlInputLocator = await getPropControlInputLocator(editorModel, 'label');
await labelControlInputLocator.fill('');
await labelControlInputLocator.fill(TEST_VALUE_2);

await expect(firstInputLabelLocator).toHaveText(TEST_VALUE_2);
await inputByLabel.waitFor({ state: 'visible' });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we're not checking if the value in the label matches in the end anymore? Was there any issue with that?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes we are, if the value doesn't match this selector wouldn't locate elements

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh it checks with that value already, i missed that. looks good then!

});