Skip to content

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

Draft
wants to merge 23 commits into
base: develop
Choose a base branch
from

Conversation

somebody1234
Copy link
Contributor

@somebody1234 somebody1234 commented Jun 14, 2025

Pull Request Description

Warning

Currently needs to be updated using the most recent suggestions in #13217

  • Close IDE Codebase structure #13217
    • Move dashboard/components/ to react-components/
    • Move dashboard/authentication/ to authentication/
    • Move dashboard/layouts/Settings/ to settings/
    • Move */Drive/ to data-catalog/
    • Move dashboard/services/ to services/
    • Move some hooks/ and modals/ to more appropriate locations
    • Remove unused assets/
    • Update 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.
  • All code follows the
    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.

@somebody1234 somebody1234 added CI: No changelog needed Do not require a changelog entry for this PR. x-chore Type: chore labels Jun 14, 2025
@somebody1234 somebody1234 requested a review from Frizi as a code owner June 14, 2025 11:07
Copy link

github-actions bot commented Jun 14, 2025

✨ GUI Checks Results

Summary

  • 🧹 Prettier: ❌ Failed
  • 🧰 GUI Checks: ❌ Failed
  • 📚 Storybook: ✅ Deployed

⚠️ Action Required: Please review the failed checks and fix them before merging.

See individual check results for more details.

ℹ️ Chromatic Tests Skipped

Chromatic visual regression tests were skipped for this PR.

To run the tests and deploy Storybook, add the CI: Run Chromatic label to this PR.

Note: We skip tests by default to optimize CI costs. Only run them when your UI changes are ready for review.

👮 Lint GUI Results

Check Results

  • 🧠 Typecheck: ✅ Passed
  • 🧹 Lint: ✅ Passed
  • 🧪 Unit Tests: ❌ Failed

🎭 Playwright Test Results

Summary

  • ⏳ Duration: 3m 18s
  • ✅ Passed: 0 tests
  • 🔴 Failed: 0 tests
  • ⚠️ Flaky: 0 tests

📥 Download Detailed Report

All tests passed! Your changes look good.

@somebody1234 somebody1234 marked this pull request as draft June 16, 2025 09:51
@somebody1234 somebody1234 marked this pull request as ready for review June 16, 2025 10:26
Copy link
Contributor

@farmaazon farmaazon left a 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 in pages subdir?
  • settings, data-catalog, (and consequently project-view) could be inside tabs or grouped with AppContainer (so we have AppContainer dir with Settings, DataCatalog, ProjectView dirs and AppContainer.vue, index.ts files.

Let others also speak.

@somebody1234
Copy link
Contributor Author

yup, that makes sense. also of note: i want to move Editor.tsx somewhere more appropriate (and perhaps ideally it should be phased out entirely, as is it is an unnecessary React layer between the Vue root and the Vue project-view)

@somebody1234
Copy link
Contributor Author

somebody1234 commented Jun 16, 2025

other notes:

  • dashboard/utilities/, project-view/util/, utils/, and enso-common are not being merged in this PR as i think that is a much more complex topic that requires further discussion

@somebody1234
Copy link
Contributor Author

update: yes the new structure is very flat, do appreciate more discussion though.
my goals for this restructure:

  • remove dashboard/ as a top level concept
  • (less important) reduce unnecessary nesting - 1-2 levels on average is much better than 3-5 levels

@somebody1234 somebody1234 marked this pull request as draft June 23, 2025 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: No changelog needed Do not require a changelog entry for this PR. g-dashboard x-chore Type: chore
Projects
None yet
Development

Successfully merging this pull request may close these issues.

IDE Codebase structure
2 participants