-
Notifications
You must be signed in to change notification settings - Fork 323
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
Dashboard improvements 08/26/2024 #10946
Conversation
2, 3, 5: will do. for 3... i think maybe this should be fixed on the BE side? might need further discussion |
2, 5 fixed, as mentioned above i'm not sure about 3. maybe it should be fixed separately. |
ok lets discuss trash on channel ✅ |
@@ -74,9 +76,28 @@ export default class LocalStorage { | |||
|
|||
/** Register runtime behavior associated with a {@link LocalStorageKey}. */ | |||
static registerKey<K extends LocalStorageKey>(key: K, metadata: LocalStorageKeyMetadata<K>) { | |||
if (!IS_DEV_MODE) { | |||
invariant( |
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.
this one looks... silly. I do want to know that before we deploy to prod :)
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.
ah, good point... i was hiding it because it was (incorrectly) showing up on HMR refresh. any thoughts? i could instead have it e.g. throw and error and catch it to capture the stack trace... it's hacky but should more or less work...
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.
Maybe we can utilize this API to avoid incorrect behavior during HMR? https://vitejs.dev/guide/api-hmr
But this might not be sufficient: we need to make sure that the key is exact the same. (the key is coming from the same module)
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.
not sure it's an option, yeah. especially since (afaict) you'd have to pass in import.meta.hot
from the module that is calling .register
, which means exposing implementation details to implementors.
right now i'm pushing an implementation based on stack traces instead, let me know your thoughts i guess?
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.
In dev mode, I'm okay with that.
FWIW, to get access to the stack trace, you can call new Error().stack
instead
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.
yeah XD i was throwing a new error and then wondering why i was catching it when i could just access its stack directly
- Address part of enso-org/cloud-v2#1453 - Many of the other issues are addressed in other PRs. - Username in "Set username" dialog defaults to user email - Hide "Share with" column for Free and Solo plans - Reorder categories in left sidebar - Previous: Cloud, My Files, Recent, Trash, ...Users, ...Teams - New: Cloud, Me (formerly My Files), ...Users, Teams, Recent, Trash - Fix #10968 - Show column toggles on Local category too (previously was explicitly specialcased to only be visible on Cloud categories as a remnant of the old design with the backend switcher at the top) - Added to this PR as this PR already touches the column toggles None
- Address part of enso-org/cloud-v2#1453 - Many of the other issues are addressed in other PRs. - Username in "Set username" dialog defaults to user email - Hide "Share with" column for Free and Solo plans - Reorder categories in left sidebar - Previous: Cloud, My Files, Recent, Trash, ...Users, ...Teams - New: Cloud, Me (formerly My Files), ...Users, Teams, Recent, Trash - Fix #10968 - Show column toggles on Local category too (previously was explicitly specialcased to only be visible on Cloud categories as a remnant of the old design with the backend switcher at the top) - Added to this PR as this PR already touches the column toggles # Important Notes None (cherry picked from commit 3420a05)
) Closes: enso-org/cloud-v2#1473 This PR fixes a bug introduced in #10946
- Address part of enso-org/cloud-v2#1453 - Many of the other issues are addressed in other PRs. - Username in "Set username" dialog defaults to user email - Hide "Share with" column for Free and Solo plans - Reorder categories in left sidebar - Previous: Cloud, My Files, Recent, Trash, ...Users, ...Teams - New: Cloud, Me (formerly My Files), ...Users, Teams, Recent, Trash - Fix #10968 - Show column toggles on Local category too (previously was explicitly specialcased to only be visible on Cloud categories as a remnant of the old design with the backend switcher at the top) - Added to this PR as this PR already touches the column toggles # Important Notes None (cherry picked from commit 3420a05)
) Closes: enso-org/cloud-v2#1473 This PR fixes a bug introduced in #10946 (cherry picked from commit 1fdaf37)
- Address part of enso-org/cloud-v2#1453 - Many of the other issues are addressed in other PRs. - Username in "Set username" dialog defaults to user email - Hide "Share with" column for Free and Solo plans - Reorder categories in left sidebar - Previous: Cloud, My Files, Recent, Trash, ...Users, ...Teams - New: Cloud, Me (formerly My Files), ...Users, Teams, Recent, Trash - Fix #10968 - Show column toggles on Local category too (previously was explicitly specialcased to only be visible on Cloud categories as a remnant of the old design with the backend switcher at the top) - Added to this PR as this PR already touches the column toggles # Important Notes None (cherry picked from commit 3420a05)
) Closes: enso-org/cloud-v2#1473 This PR fixes a bug introduced in #10946 (cherry picked from commit 1fdaf37)
Pull Request Description
Address part of https://github.com/enso-org/cloud-v2/issues/1453
Fix In the local projects list, columns can be removed but cannot be re-added #10968
Important Notes
None
Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
The documentation has been updated, if necessary.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.