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

[docs] Add CRUD to themed example #4785

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

bharatkashyap
Copy link
Member

@bharatkashyap bharatkashyap commented Mar 19, 2025

  • Update "auth-nextjs-themed" to include Crud
  • Merge the same updates to the hosted template once this is merged

@bharatkashyap bharatkashyap added docs Improvements or additions to the documentation examples Relating to /examples labels Mar 19, 2025
@mui-bot
Copy link

mui-bot commented Mar 19, 2025

Netlify deploy preview

https://deploy-preview-4785--mui-toolpad-docs.netlify.app/

Generated by 🚫 dangerJS against 4d3fd84

}, [listPath, router]);

return (
<CrudProvider<Employee> dataSource={employeesDataSource} dataSourceCache={employeesCache}>
Copy link
Member

@apedroferreira apedroferreira Mar 19, 2025

Choose a reason for hiding this comment

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

I'll expose the dataGrid slot to the Crud component so we can just use that component here.

Copy link
Member

@apedroferreira apedroferreira Mar 19, 2025

Choose a reason for hiding this comment

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

Feel free to just add a TODO comment here for this for now as i guess we will only be able to adjust here after the next release once we merge #4786

Copy link
Member

@apedroferreira apedroferreira left a comment

Choose a reason for hiding this comment

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

Looks good! Just noticed a few things that probably we can improve:

  1. Add:
import PersonIcon from '@mui/icons-material/Person';

// ...
 {
    segment: 'employees',
    title: 'Employees',
    icon: <PersonIcon />,
    pattern: 'employees{/:employeeId}*',
  },
// ...

to the navigation to show employees pages and breadcrumbs.

  1. There's no spacing above and below the action icon buttons, not sure if we should solve that in the template itself or in the CRUD components? (just checked and all looks fine in the standard version so I guess we can fix it somehow for this template)
Screenshot 2025-03-19 at 14 35 28
  1. The form input labels need spacing below, I guess this one should probably be fixed in the template?
Screenshot 2025-03-19 at 14 34 33

@bharatkashyap
Copy link
Member Author

bharatkashyap commented Mar 20, 2025

Looks good! Just noticed a few things that probably we can improve:

Thanks! Addressed all of this in the theme of the template

@apedroferreira
Copy link
Member

Thanks for the changes!
I still found a few issues now and I guess a few are blocking, listing them all here:

  1. The input labels are still touching the inputs, they might need a margin or padding at the bottom?
Screenshot 2025-03-21 at 17 17 44
  1. Getting an error when opening the orders page:
Screenshot 2025-03-21 at 17 17 55
  1. This one isn't as important, but I tried to make it so that the CRUD made the mouse a "cursor"/hand when hovering over the list rows, to make it more obvious that they're links, but this behavior isn't happening in this themed example. If it's an easy fix feel free to add it, but it was just something else I noticed.
Screen.Recording.2025-03-21.at.17.18.54.mov

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation examples Relating to /examples
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants