search

mui / mui-toolpad

Toolpad Studio: Low-code admin builder. Open-source and powered by MUI.
https://mui.com/toolpad/
MIT License
815 stars 206 forks source link

Add app navigation sidebar #1819

Closed apedroferreira closed 1 year ago

apedroferreira commented 1 year ago

Closes https://github.com/mui/mui-toolpad/issues/813 Closes https://github.com/mui/mui-toolpad/issues/1547

Development:

https://user-images.githubusercontent.com/10789765/228324130-b19d2f85-addc-4831-aa35-e72568b860a7.mov

Production

https://user-images.githubusercontent.com/10789765/228049624-4196296f-40ed-406d-aaba-f7fe3b5758ae.mov

prakhargupta1 commented 1 year ago

Nice! Is it that both preview options (1. right panel and 2. top bar) show the same result i.e, the preview for the entire app? If Yes, then I think we can remove the one in the right bar.

If we want to make toolpad-display=standalone more discoverable, we can also put page preview available from the ellipsis menu of the page.

apedroferreira commented 1 year ago

Nice! Is it that both preview options (1. right panel and 2. top bar) show the same result i.e, the preview for the entire app? If Yes, then I think we can remove the one in the right bar.

The option at the top opens the app in preview but starting in the default page, while the right-side option opens the app starting in that specific page. So I think it's fine to keep these separate options?

If we want to make toolpad-display=standalone more discoverable, we can also put page preview available from the ellipsis menu of the page.

I can add a "Preview" option from the ellipsis menu of each page, but I feel like it should open without toolpad-display=standalone so that the user can navigate from there if they want to? So it would be exactly the same as the option in the right-side panel.

Janpot commented 1 year ago

The option at the top opens the app in preview but starting in the default page, while the right-side option opens the app starting in that specific page. So I think it's fine to keep these separate options?

Intuitively what makes most sense to me is for the top button to open the current page and to remove the button on the right.

If we want to make toolpad-display=standalone more discoverable, we can also put page preview available from the ellipsis menu of the page.

As a start I would propose to just document this option in the docs and nothing more

apedroferreira commented 1 year ago

Intuitively what makes most sense to me is for the top button to open the current page and to remove the button on the right.

Agreed, didn't consider that but seems more intuitive, I'll do that instead.

Janpot commented 1 year ago

Would it make sense for the "edit" button on the Toolpad application to do the reverse operation? i.e. open the page editor of the page you were viewing.

prakhargupta1 commented 1 year ago

Would it make sense for the "edit" button on the Toolpad application to do the reverse operation? i.e. open the page editor of the page you were viewing.

Yes, I think that would make sense. Currently, it shows the first page in edit mode.

Janpot commented 1 year ago

I was thinking, this will break all embeds that are currently in notion already.

Our options:

  1. Do nothing in Toolpad. Update all embeds in notion
  2. Automatically set toolpad-display=standalone when an iframe is detected. use ?toolpad-display=shell to enable the navigation
  3. Provide a page-level setting that sets the default for toolpad-display
  4. Provide an app-level setting that sets the default for toolpad-display

Personally my preference goes out to 3. Would it be hard to implement? I'm fine dealing with this in a separate PR, but we need to prioritize it then so that it makes it in the same release as this one.

apedroferreira commented 1 year ago

I was thinking, this will break all embeds that are currently in notion already. Our options:

I was thinking option 1 for now but 2 or 3 also seem fine. Let's do 3 then, I'll try to include it in this PR as I think it shouldn't be that difficult.

bharatkashyap commented 1 year ago

I was thinking, this will break all embeds that are currently in notion already.

Our options:

  1. Do nothing in Toolpad. Update all embeds in notion
  2. Automatically set toolpad-display=standalone when an iframe is detected. use ?toolpad-display=shell to enable the navigation
  3. Provide a page-level setting that sets the default for toolpad-display
  4. Provide an app-level setting that sets the default for toolpad-display

Personally my preference goes out to 3. Would it be hard to implement? I'm fine dealing with this in a separate PR, but we need to prioritize it then so that it makes it in the same release as this one.

For me, an app-level setting is useful for this since it affects all pages. I was also wondering whether this is a good way to introduce "layouts" to the app: You can start with layout option 1 (a sidebar for navigation), or 2 (full page content, and a separate page for navigation) and then in the future, potentially, option 3 (tabs for navigation?), option 4 ...

apedroferreira commented 1 year ago

I was thinking, this will break all embeds that are currently in notion already. Our options:

  1. Do nothing in Toolpad. Update all embeds in notion
  2. Automatically set toolpad-display=standalone when an iframe is detected. use ?toolpad-display=shell to enable the navigation
  3. Provide a page-level setting that sets the default for toolpad-display
  4. Provide an app-level setting that sets the default for toolpad-display

Personally my preference goes out to 3. Would it be hard to implement? I'm fine dealing with this in a separate PR, but we need to prioritize it then so that it makes it in the same release as this one.

For me, an app-level setting is useful for this since it affects all pages. I was also wondering whether this is a good way to introduce "layouts" to the app: You can start with layout option 1 (a sidebar for navigation), or 2 (full page content, and a separate page for navigation) and then in the future, potentially, option 3 (tabs for navigation?), option 4 ...

I just implemented option 3 but if we see it doesn't feel that useful we can try the other ones... I guess it would also depend on people's use cases, whether anyone would like different display settings per page or most people would use the same for every page.

Also this could definitely support different layouts / navigation UIs.

Janpot commented 1 year ago

I'm not sure the app-wide setting would be the best solution for us. The tools-public application has pages that we need to be chromeless, as well as pages that we need to have the sidebar.

apedroferreira commented 1 year ago

I'm not sure the app-wide setting would be the best solution for us. The tools-public application has pages that we need to be chromeless, as well as pages that we need to have the sidebar.

Ok that's good to hear, I guess option 3 should be good then.

apedroferreira commented 1 year ago

I've implemented option 3 as discussed:

https://user-images.githubusercontent.com/10789765/229141902-f7f4f0c4-8a6f-4ab0-8d01-dd6dd2406faa.mov

Let me know if you have any more feedback or I can merge it later today or early next week!