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

Optimize assets table #10522

Merged
merged 14 commits into from
Jul 15, 2024
Merged

Optimize assets table #10522

merged 14 commits into from
Jul 15, 2024

Conversation

somebody1234
Copy link
Contributor

Pull Request Description

Other misc fixes:

  • Fix "unique key prop" error in sidebar
  • Vertically center asset panel toggle icon in top bar
  • Limit width of asset names (show tooltip when asset name is too wide)

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.

@somebody1234 somebody1234 added p-high Should be completed in the next sprint g-dashboard x-refactor Changes that should not be visible to the end-user labels Jul 11, 2024
@somebody1234 somebody1234 added the CI: No changelog needed Do not require a changelog entry for this PR. label Jul 11, 2024
// ===============
// === Newtype ===
// ===============

/** An interface specifying the variant of a newtype. */
export interface NewtypeVariant<TypeName extends string> {
type NewtypeVariant<TypeName extends string> = {
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume you want to forbid the flexibility which interface gives us?

Also, the types are no longer exported; are they actually used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

forbid the flexibility which interface gives us?

i guess technically? but it's not the main reason

the types are no longer exported; are they actually used?

only internally - this is the real reason behind this change. apparently if we don't export the interfaces, TS complains because it cannot name them (because interface declarations are kinda sorta nominal because they can be merged into)

so basically this change is so that we no longer have to export it (and clutter up autocomplete) just so that it's visible

@PabloBuchu
Copy link
Contributor

@somebody1234 Is there any particular thing that should be QAed or just general performance improvements?

@somebody1234 somebody1234 added the CI: Ready to merge This PR is eligible for automatic merge label Jul 15, 2024
@mergify mergify bot merged commit 1514381 into develop Jul 15, 2024
37 of 41 checks passed
@mergify mergify bot deleted the wip/sb/remove-event-list branch July 15, 2024 10:06
@somebody1234
Copy link
Contributor Author

yeah it should be just perf improvements. specifically the only thing (other than minor CSS improvements) is a refactor of the "reactive events" that were previously being used to minimize the amount of things that need to re-render

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. CI: Ready to merge This PR is eligible for automatic merge g-dashboard p-high Should be completed in the next sprint x-refactor Changes that should not be visible to the end-user
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants