[WIP] Refactor layout #116

eneufeld · 2020-12-21T17:12:30Z

No description provided.

sdirix

I like this change in general!

Some things I noticed:

This looks rather empty, maybe we should add text here like "Panel"?
Left panel:
- As we currently need the panel to do any editing it should be open by default
- Don't show the icons when closed as clicking them does nothing. Alternatively they should also open the drawer.
- The ui schema icon is very misleading
The vertical scrollbar on the left panel is cut off at top and bottom
When forcing the editor into a horizontal scroll, for example by using a horizontal layout with many controls an additional vertical scrollbar for the whole JSON Forms editor appears. This was not the case in the old editor.
There should be a title for each panel category

Also:

Not part of this PR but the Angular preview doesn't show anything when ui schema is empty
We should remove commented code

sdirix · 2020-12-22T20:43:44Z

app/src/App.tsx

@@ -22,7 +22,7 @@ export const App = () => (
    schemaService={schemaService}
    schemaProviders={defaultSchemaProviders}
    schemaDecorators={defaultSchemaDecorators}
-    editorTabs={[
+    previewTabs={[


sdirix · 2020-12-22T22:10:00Z

jsonforms-editor/src/palette-panel/components/PalletePanel.tsx

+          <ListItem
+            button
+            onClick={() => handleTabChange(0)}
+            selected={selectedTab === 0}
+          >
+            <ListItemIcon data-cy='palette-tab'>
+              <BallotIcon />
+            </ListItemIcon>
+          </ListItem>
+          <ListItem
+            button
+            onClick={() => handleTabChange(1)}
+            selected={selectedTab === 1}
+          >


Maybe we can encapsulate the index handling into a reusable component, similar to the other panels?

sdirix · 2020-12-22T22:11:02Z

jsonforms-editor/src/properties/components/PropertiesPanel.tsx

+      <Tabs value={selectedTab} onChange={handleTabChange}>
+        {previewTabs
+          ? previewTabs.map((tab) => (
+              <Tab key={`tab-${tab.name}`} label={tab.name} />
+            ))
+          : null}


Do we need these ternaries? Isn't the map enough?

sdirix · 2020-12-22T22:12:17Z

jsonforms-editor/src/palette-panel/components/PalletePanel.tsx

+    width: theme.spacing(7) + 1,
+    maxWidth: theme.spacing(7) + 1,
+    [theme.breakpoints.up('sm')]: {
+      width: theme.spacing(9) + 1,
+      maxWidth: theme.spacing(9) + 1,
+    },


why the +1?

sdirix · 2020-12-22T22:13:43Z

jsonforms-editor/src/editor/components/preview/ReactMaterialPreview.tsx

@@ -19,6 +19,7 @@ import { useExportSchema, useExportUiSchema } from '../../../core/util/hooks';
 import { previewOptions } from './options';

 export const ReactMaterialPreview: React.FC = () => {
+  console.log('IM RENDERER');


sdirix · 2021-03-05T09:31:05Z

Some feedback, solely based on the Preview without looking at the code. Looking at it with the eyes of a first time user:

I would like to suggest open the Palette by default. It offers a natural starting point
Would be great to have the Palette title next to the expand symbol when expanded.
We should start with a simple ui schema already loaded. This eases the starting experience.
When there is no ui schema the React preview shows the default generated one while the Angular preview doesn't show anything. They should be aligned
When there is no ui schema it should be indicated to the user in some way that the default generated one is used
When there is no ui schema we should offer a button to default generate one
It would be great to have tooltips on the palette buttons so they indicate what they are without clicking them
The textual schema and ui schema should be removed from the Palette, as they don't really belong there and clutter the interface. Suggestion: Add a gear icon next to the download icon and let the user edit them in a modal when necessary.
The download button should show a menu of two entries to allow "downloading" a schema and ui schema json file. If this is to much effort we can also just remove it and only show the gear icon mentioned above.

Of course all of these are optional however I think they would improve the user experience. I think as a long as the inline editing is not 100% perfect we should keep the palette (so probably for a very long time) and polish it accordingly too.

sdirix · 2021-03-05T09:37:51Z

Hmm using the editor for a while: The properties view is too inaccessible. We should maybe permanently show it in some way. For example when I want to modify a label I created inline I have to open the palette and switch to it. This is way too cumbersome.

Just an idea: Maybe merge it with the Layout/Control page, e.g. always show it on 50% height? When we additionally remove the schema/ui schema entries then there is only this single palette panel left. Then drag and dropping in an element would require no further clicks to edit its properties.

koegel · 2021-03-05T10:06:31Z

Before I drop off even more "complains", I like the idea to make the editor more approachable to users :)

Improvement ideas:

OnMouseOver Tooltips for the icons in the panel
Panel should be expanded on UI Schema on load (agree with Stefan)
Where is the property view to edit properties, e.g. of Label ?

Please let me know when there is an update, I would like to review again

eneufeld · 2021-03-05T23:19:21Z

Thank you for the feedback.
From the comments I see three groups of issues:

follow up issues

We should start with a simple ui schema already loaded. This eases the starting experience.
When there is no ui schema the React preview shows the default generated one while the Angular preview doesn't show anything. They should be aligned
When there is no ui schema it should be indicated to the user in some way that the default generated one is used
When there is no ui schema we should offer a button to default generate one
The textual schema and ui schema should be removed from the Palette, as they don't really belong there and clutter the interface. Suggestion: Add a gear icon next to the download icon and let the user edit them in a modal when necessary.
The download button should show a menu of two entries to allow "downloading" a schema and ui schema json file. If this is to much effort we can also just remove it and only show the gear icon mentioned above.

low hanging fruits

OnMouseOver Tooltips for the icons in the panel
This will solve
- Where is the property view to edit properties, e.g. of Label ?
Would be great to have the Palette title next to the expand symbol when expanded.
It would be great to have tooltips on the palette buttons so they indicate what they are without clicking them

issues that need a decision

I would like to suggest open the Palette by default. It offers a natural starting point
Panel should be expanded on UI Schema on load (agree with Stefan)
Here we need to decide whether to go with the simple solution of showing the panel or spent a bit more time and add inline editing for controls which would support selecting the scope and setting the label.

I would first fix issues in group 2. In the meantime I would love to hear which solution for group 3 you would prefer and whether creating follow up issues from group 1 is fine with you.

koegel · 2021-03-06T00:03:13Z

I would first fix issues in group 2. In the meantime I would love to hear which solution for group 3 you would prefer and whether creating follow up issues from group 1 is fine with you.

+1 on how to proceed with groups 2 and 1. For group 3 a property view in the panel is good enough for now imho.

eneufeld · 2021-03-07T11:27:51Z

Group 2 issues should be fixed.
For Group 3 I added inline editing :-P as it was simple to do.
~~The label is missing the inline editing,~~ I hope I didn't miss any controls.

- allow to modify scope of controls - allow to modify label if controls - allow to modify label of groups - allow to modify label of labels

koegel · 2021-03-08T14:09:58Z

I (also) think Palette should be open by default.
The property view is shown with a horizontal scrollbar for me.

eneufeld · 2021-03-08T14:28:38Z

I (also) think Palette should be open by default.

Why do you need the palette with the inline editing available?

koegel · 2021-03-10T16:31:56Z

I (also) think Palette should be open by default.

Why do you need the palette with the inline editing available?

I personally feel the palette is more approachable to new users because I can see my options right away, but I might be wrong.

eneufeld requested a review from sdirix December 21, 2020 17:12

eneufeld force-pushed the layout-refactor branch from 92f4165 to 61de15a Compare December 22, 2020 21:42

sdirix reviewed Dec 22, 2020

View reviewed changes

[WIP] Refactor layout

abcd1aa

eneufeld force-pushed the layout-refactor branch 2 times, most recently from 2486fb5 to 73186af Compare March 4, 2021 21:49

Add inline button for add operation

62a7d8d

eneufeld force-pushed the layout-refactor branch from 73186af to 62a7d8d Compare March 6, 2021 13:53

Add support for inline editing

722dbc5

- allow to modify scope of controls - allow to modify label if controls - allow to modify label of groups - allow to modify label of labels

eneufeld force-pushed the layout-refactor branch from 6372950 to 722dbc5 Compare March 7, 2021 11:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Refactor layout #116

[WIP] Refactor layout #116

eneufeld commented Dec 21, 2020

sdirix left a comment •

edited

Loading

sdirix Dec 22, 2020

sdirix Dec 22, 2020

sdirix Dec 22, 2020

sdirix Dec 22, 2020

sdirix Dec 22, 2020

sdirix commented Mar 5, 2021 •

edited

Loading

sdirix commented Mar 5, 2021 •

edited

Loading

koegel commented Mar 5, 2021

eneufeld commented Mar 5, 2021

koegel commented Mar 6, 2021

eneufeld commented Mar 7, 2021 •

edited

Loading

koegel commented Mar 8, 2021

eneufeld commented Mar 8, 2021

koegel commented Mar 10, 2021

[WIP] Refactor layout #116

Are you sure you want to change the base?

[WIP] Refactor layout #116

Conversation

eneufeld commented Dec 21, 2020

sdirix left a comment • edited Loading

Choose a reason for hiding this comment

sdirix Dec 22, 2020

Choose a reason for hiding this comment

sdirix Dec 22, 2020

Choose a reason for hiding this comment

sdirix Dec 22, 2020

Choose a reason for hiding this comment

sdirix Dec 22, 2020

Choose a reason for hiding this comment

sdirix Dec 22, 2020

Choose a reason for hiding this comment

sdirix commented Mar 5, 2021 • edited Loading

sdirix commented Mar 5, 2021 • edited Loading

koegel commented Mar 5, 2021

eneufeld commented Mar 5, 2021

koegel commented Mar 6, 2021

eneufeld commented Mar 7, 2021 • edited Loading

koegel commented Mar 8, 2021

eneufeld commented Mar 8, 2021

koegel commented Mar 10, 2021

sdirix left a comment •

edited

Loading

sdirix commented Mar 5, 2021 •

edited

Loading

sdirix commented Mar 5, 2021 •

edited

Loading

eneufeld commented Mar 7, 2021 •

edited

Loading