-
Notifications
You must be signed in to change notification settings - Fork 330
Restructure dashboard codebase #13278
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
base: develop
Are you sure you want to change the base?
Conversation
✨ GUI Checks ResultsSummary
See individual check results for more details. ℹ️ Chromatic Tests SkippedChromatic visual regression tests were skipped for this PR. To run the tests and deploy Storybook, add the Note: We skip tests by default to optimize CI costs. Only run them when your UI changes are ready for review. 👮 Lint GUI ResultsCheck Results
🎭 Playwright Test ResultsSummary
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The strcuture is a bit "too flat" to my liking.
- personally, I would not mix logic with display. I would see
authentication
to have mostly "logic for authentication" (cognito & co.). Most of the moved react components are actually pages, so maybe put them inpages
subdir? - settings, data-catalog, (and consequently project-view) could be inside
tabs
or grouped withAppContainer
(so we haveAppContainer
dir withSettings
,DataCatalog
,ProjectView
dirs andAppContainer.vue
,index.ts
files.
Let others also speak.
yup, that makes sense. also of note: i want to move |
other notes:
|
update: yes the new structure is very flat, do appreciate more discussion though.
|
Pull Request Description
Warning
Currently needs to be updated using the most recent suggestions in #13217
dashboard/components/
toreact-components/
dashboard/authentication/
toauthentication/
dashboard/layouts/Settings/
tosettings/
*/Drive/
todata-catalog/
dashboard/services/
toservices/
hooks/
andmodals/
to more appropriate locationsassets/
CODEOWNERS
Important Notes
None
Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
The documentation has been updated, if necessary.Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.Scala,
Java,
TypeScript,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
Unit tests have been written where possible.If meaningful changes were made to logic or tests affecting Enso Cloud integration in the libraries,or the Snowflake database integration, a run of the Extra Tests has been scheduled.
If applicable, it is suggested to paste a link to a successful run of the Extra Tests.