New page creation UX #2728

apedroferreira · 2023-09-27T15:36:34Z

File explorer-like creation UX for new pages, similar to what we have / will have for functions.
Ability to rename pages and page hierarchy elements by double-clicking, using the same logic/component (will also have this later in functions)
Sticky headers for these left-side panels, as opposed to the previous collapsible headers, and adjusted spacing/sizing

Screen.Recording.2023-10-04.at.17.40.18.mov

Janpot · 2023-09-28T07:32:46Z

cc @flaviendelangle might be interested in this use case, for an x-tree-view demo or for pro features.

create/edit tree items + validation
scroll into view after creation

packages/toolpad-app/src/toolpad/AppEditor/PagesExplorer/index.tsx

packages/toolpad-app/src/components/CreateTreeItem.tsx

mbrookes · 2023-09-29T18:21:43Z

cc @flaviendelangle might be interested in this use case, for an x-tree-view demo or for pro features.

What's the reason for using a TreeView in the first place given that pages can't be nested, and collapsing the top-level doesn't seem useful?

…/improvements/refactors

Janpot · 2023-09-30T12:37:37Z

What's the reason for using a TreeView in the first place given that pages can't be nested, and collapsing the top-level doesn't seem useful?

We're looking for a UX similar to the vscode file explorer. The data roughly maps on the file system and has similar functionality around create/rename. It's true that pages can't be nested (yet), we do plan to use this pattern in some places in our UI which will have nested items. So far the other places have 1 level of nesting (function files with exported functions). It could be valuable to explore whether there are other UI patterns than a tree view to represent these.

We should probably remove the top-level "Pages" item. This is a leftover from a time when there were more top-level items.

…ader component for explorer panels

Janpot · 2023-10-05T09:18:05Z

packages/toolpad-app/src/components/ExplorerHeader.tsx

If it's for the Hierarchy explorer, then I think this file can go under the toolpad/AppEditor/HierarchyExplorer folder. Or under the shared common ancestor.
The philosphy behind the components folder is to use it for general purpose components. Basically, the missing components from MUI core

No, I'm using this component in both the hierarchy explorer and the pages explorer. It's supposed to be the header for any "explorer" panel like this.
Eventually I can make these components part of a larger one that handles any explorer panel including the header and creatable/renamable items. I guess I wanted to keep it more modular first. But maybe I should try it already.

Oh, I can try to move the component above just enough in the project structure probably if it makes sense, might be better than in the components folder then.

Janpot · 2023-10-05T09:26:11Z

packages/toolpad-app/src/toolpad/AppEditor/PagesExplorer/index.tsx

+ sx={pagesTreeItemSx}
+ />
+ ) : null}
+ {sortBy(pages, 'name').map((page) => (


This is mutating the pages array. Best to Avoid mutating variables in the render function.
It's also running a sort i every render. Feels a bit wasteful.

Yeah, I noticed it was mutable, in this particular case it doesn't have too much impact but it's better to make it immutable and memoize it, I'll do it.

Janpot · 2023-10-05T09:29:04Z

packages/toolpad-utils/src/collections.ts

+ * Sorts an array of objects by the value of one property.
+ */
+export function sortBy<P extends object>(objs: P[], prop: keyof P, isAsc = true): P[] {
+ return objs.sort((a, b) => {


None of the other exports of this file mutate the source array. Feels a bit counterintuitive, and I feel like it could lead to bugs. An alternative could be to add a

export function createPropertyComparator<T>(propName: string): (a: T, b: T) => number;

and let the consumer decide whether they want to mutate their array or not.

I think it's ok to just make this method immutable, simpler and more intuitive. At first I guess I wanted it to be more like .sort, but the way that we use it it's probably better for it just to be immutable.

Janpot · 2023-10-05T09:30:35Z

packages/toolpad-app/src/utils/scroll.ts

How about a utils/dom.ts to be a bit broader?

That's fine, I wasn't really sure where to put this.

Janpot

Looks good. I proposed a few tweaks regarding project organization. Other remarks:

🤔 For visual consistency, I feel like it would be better if these are aligned
Will it be confusing to remove the existing page name field? It's a bit obsolete now, but I feel like we could still offer a way to edit the name in that page options panel. Perhaps less obtrusive as the one we have now. In any case, we can keep it for a follow up PR if we leave the name existing editor in place.

apedroferreira · 2023-10-05T13:43:21Z

🤔 For visual consistency, I feel like it would be better if these are aligned

The issue is that in the hierarchy explorer we need space for the little expand/collapse arrow for nestable elements. I'll see if it works to add some more padding to the pages though, might be fine.

Will it be confusing to remove the existing page name field? It's a bit obsolete now, but I feel like we could still offer a way to edit the name in that page options panel. Perhaps less obtrusive as the one we have now. In any case, we can keep it for a follow up PR if we leave the name existing editor in place.

I've actually already removed it... There's pros and cons to removing it or not, on one hand it gives the users maybe a more obvious place to change the name, but on the other hand it might be confusing how it's the same thing as renaming on the left. And it might be easier for users to associate renaming in the left panel with renaming the project files. I guess I can keep the input for now and we'll see how things go from there.

apedroferreira · 2023-10-05T19:46:36Z

I've added all the changes discussed. Updated UI screenshot:

Janpot · 2023-10-06T10:32:13Z

packages/toolpad-app/src/toolpad/AppEditor/PagesExplorer/index.tsx

- ml: -1.5,
- },
- };
+ const alphabeticallySortedPages = React.useMemo(() => sortBy(pages, 'name'), [pages]);


That won't sort alphabetically. e.g. 'a' < 'B' === false.

You might want to use .localeCompare instead:

[...pages].sort((page1, page2) => page1.name.localeCompare(page2.name)

Or if we abstract it I'd do it as a collator

interface Collator<T> { (a: T, b: T): number } function createByPropCollator<T, K extends keyof T>(propName: K, collator?: Collator<T[K]>): Collator<T>;

Typesafe, composable, easy to use with both .sort and upcoming .toSorted

const pagesCollator = createByPropCollator<Page>('name', createAlphabeticCollator()) pages.toSorted(pagesCollator)

Uh, that's right about the alphabetical ordering... And nice, I didn't know about localeCompare or toSorted.
The first approach seems fine, I tried implementing the second abstraction you proposed but I'm not really sure how to? And we might not need that abstraction yet anyway...

I've tried adding these as comparators, let me know what you think and feel free to provide more feedback.

Janpot

Great, thanks for updating!

Janpot · 2023-10-06T23:42:00Z

@apedroferreira The position of the scrollbar seems to be off on the pages section. I don't believe position: sticky/fixed should be used for headers. The UX feels off when the scrollbar extends over the header.

New page creation UX

654d654

apedroferreira added the enhancement This is not a bug, nor a new feature label Sep 27, 2023

apedroferreira self-assigned this Sep 27, 2023

apedroferreira marked this pull request as draft September 27, 2023 16:04

fix tests

72f1b43

apedroferreira marked this pull request as ready for review September 27, 2023 17:43

Remove .only

690c38a

Janpot reviewed Sep 28, 2023

View reviewed changes

packages/toolpad-app/src/toolpad/AppEditor/PagesExplorer/index.tsx Outdated Show resolved Hide resolved

Janpot reviewed Sep 28, 2023

View reviewed changes

packages/toolpad-app/src/toolpad/AppEditor/PagesExplorer/index.tsx Outdated Show resolved Hide resolved

Janpot reviewed Sep 28, 2023

View reviewed changes

packages/toolpad-app/src/toolpad/AppEditor/PagesExplorer/index.tsx Outdated Show resolved Hide resolved

apedroferreira added 5 commits September 28, 2023 21:59

Address PR feedback

de0fc98

Improvements/adjustments

b3e8517

Fix type name

95201a9

Merge remote-tracking branch 'upstream/master' into new-page-creation-ux

815c290

Better import order

9836f74

apedroferreira requested review from Janpot and bharatkashyap September 29, 2023 12:43

Janpot reviewed Sep 29, 2023

View reviewed changes

packages/toolpad-app/src/components/CreateTreeItem.tsx Outdated Show resolved Hide resolved

Change to EditableTreeItem and use for creating and renaming pages

543ca49

apedroferreira added 2 commits September 29, 2023 19:45

Use new EditableTreeItem in hierarchy explorer, with respective fixes…

0683253

…/improvements/refactors

Remove unnecessary omit

23e639d

apedroferreira added 6 commits October 2, 2023 18:54

Adjust TreeView layout and spacing for left-side explorers, create he…

76b6ff7

…ader component for explorer panels

Fix spacing and pages sorting

ba6306f

Adjust test for renaming pages

ce59515

Merge remote-tracking branch 'upstream/master' into new-page-creation-ux

ffb31aa

Remove .only

f5320be

Try to fix tests and spacing issues:

866bd60

apedroferreira added 2 commits October 3, 2023 22:17

More test adjustments

ce7f41c

Merge branch 'master' into new-page-creation-ux

ce1593e

apedroferreira requested review from Janpot and a team October 4, 2023 16:44

Janpot reviewed Oct 5, 2023

View reviewed changes

apedroferreira added 2 commits October 5, 2023 20:45

Review fixes

bdb31c7

Merge remote-tracking branch 'upstream/master' into new-page-creation-ux

42c66de

Janpot reviewed Oct 6, 2023

View reviewed changes

Janpot mentioned this pull request Oct 6, 2023

[core] Use readonly for Editable Text #2762

Closed

apedroferreira added 6 commits October 6, 2023 14:31

Fix sorting

d5594be

isSorted isn't widely available yet

e0b488f

Merge remote-tracking branch 'upstream/master' into new-page-creation-ux

f4ae372

Introduce and use comparators

67ceeb8

rewording

904c73a

Merge remote-tracking branch 'upstream/master' into new-page-creation-ux

f717443

Janpot approved these changes Oct 6, 2023

View reviewed changes

apedroferreira enabled auto-merge (squash) October 6, 2023 16:03

apedroferreira merged commit c310f41 into mui:master Oct 6, 2023
11 checks passed

apedroferreira deleted the new-page-creation-ux branch October 9, 2023 16:57

apedroferreira mentioned this pull request Oct 11, 2023

Fix creating new page with blur and default name + scroll only below explorer headers #2790

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New page creation UX #2728

New page creation UX #2728

apedroferreira commented Sep 27, 2023 •

edited

Loading

Janpot commented Sep 28, 2023 •

edited

Loading

mbrookes commented Sep 29, 2023

Janpot commented Sep 30, 2023

Janpot Oct 5, 2023 •

edited

Loading

apedroferreira Oct 5, 2023 •

edited

Loading

apedroferreira Oct 5, 2023

Janpot Oct 5, 2023

apedroferreira Oct 5, 2023

Janpot Oct 5, 2023

apedroferreira Oct 5, 2023

Janpot Oct 5, 2023

apedroferreira Oct 5, 2023

Janpot left a comment

apedroferreira commented Oct 5, 2023

apedroferreira commented Oct 5, 2023

Janpot Oct 6, 2023 •

edited

Loading

apedroferreira Oct 6, 2023

apedroferreira Oct 6, 2023

Janpot left a comment

Janpot commented Oct 6, 2023

New page creation UX #2728

New page creation UX #2728

Conversation

apedroferreira commented Sep 27, 2023 • edited Loading

Janpot commented Sep 28, 2023 • edited Loading

mbrookes commented Sep 29, 2023

Janpot commented Sep 30, 2023

Janpot Oct 5, 2023 • edited Loading

Choose a reason for hiding this comment

apedroferreira Oct 5, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Janpot left a comment

Choose a reason for hiding this comment

apedroferreira commented Oct 5, 2023

apedroferreira commented Oct 5, 2023

Janpot Oct 6, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Janpot left a comment

Choose a reason for hiding this comment

Janpot commented Oct 6, 2023

apedroferreira commented Sep 27, 2023 •

edited

Loading

Janpot commented Sep 28, 2023 •

edited

Loading

Janpot Oct 5, 2023 •

edited

Loading

apedroferreira Oct 5, 2023 •

edited

Loading

Janpot Oct 6, 2023 •

edited

Loading