fix: settings error layout
Fixes: #7460
Changes & Why
Since all the settings pages lie in the Outlet of DefaultLayout, there was no way to handle it apart from creating a separate errorFallback for the settings route. So, I created a settingsErrorFallback component that uses the same styling of settings pages. Created ErrorBoundaryWrapper that checks if its settings route then show SettingsErrorFallback else show GenericErrorFallback. Now, for the breadcrumb part. I found that all the settings pages use hardcoded title. So, I created generateBreadcrumbLinks function that will provide different title and links based on how it's respective settings page has them. If this approach looks fine, I will add the other remaining titles and links to the generateBreadcrumbLinks function and move that whole function to its separate file. And will fix linting errors.
If there is any different approach to handle it, lemme know. I'm happy to implement it.
@charlesBochet I need your review on this.
@Bonapara Should we implement this not only in settings but everywhere in the app ?
@Bonapara Should we implement this not only in settings but everywhere in the app ?
https://github.com/user-attachments/assets/c2973008-6f2e-4881-a3de-b8f9c2d10504
Don't we already do this?
Fixes: #7460
Thanks for contributing @harshrajeevsingh! Can you remove the Error Occured page title?
Fixes: #7460
Thanks for contributing @harshrajeevsingh! Can you remove the
Error Occuredpage title?
Yeah! I will remove it. @lucasbordeau any suggestion from your side? Also in which folder should I move the generateBreadcrumbLinks function after adding the other page's missing links & title?
@Bonapara Should we implement this not only in settings but everywhere in the app ?
CleanShot.2024-10-18.at.19.02.25.mp4 Don't we already do this?
@Bonapara I don't think so
Oh you're right @harshrajeevsingh, thanks.
All the designs are here:
https://www.figma.com/design/xt8O9mFeLl46C5InWwoMrN/Twenty?node-id=19439-92929&node-type=frame&t=n39yTGMmRAB8gzNq-11
@harshrajeevsingh modules/error-handlers seems like the right place ?
@Bonapara Do we need that view bar, too? I think it's better to throw errors for that whole container. If any error occurs in viewBar, too, it will show the default GenericErrorFallback. @lucasbordeau wdyt?
Hi @harshrajeevsingh, no we do not need the view bar, especially if it's a page error and not a table error
@lucasbordeau what should be passed in header of both RecordIndexError & RecordShowError fallback and how? For now, I just gave a temporary title.
/award 150
Awarding harshrajeevsingh: 150 points 🕹️ Well done! Check out your new contribution on oss.gg/harshrajeevsingh
@harshrajeevsingh Let's try to keep the breadcrumb as it was before the error occurs, and then make sure the new white background is on every error page. We don't need to spend too much time on this.
@lucasbordeau Sorry, I didn’t quite understand. Are you asking me to remove the whole breadcrumbs part and just use a white container for every page with no headings or titles?
Like this?
Yes please, also remove the breadcrumb computing part in your code, we'll see that later, thank you.
