-
Notifications
You must be signed in to change notification settings - Fork 113
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
improve query builder launch and save interactions #659
Conversation
🦋 Changeset detectedLatest commit: fbfa2ae The changes in this PR will be included in the next version bump. This PR includes changesets to release 26 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codecov Report
@@ Coverage Diff @@
## master #659 +/- ##
==========================================
- Coverage 43.61% 43.58% -0.04%
==========================================
Files 904 904
Lines 40328 40451 +123
Branches 9243 9274 +31
==========================================
+ Hits 17591 17632 +41
- Misses 22673 22750 +77
- Partials 64 69 +5
|
tabIndex={-1} | ||
title={`[${ | ||
querySetupState.toGetSnapShot ? 'on' : 'off' | ||
}] Toggle show data spaces from snapshot releases instead of latest releases`} |
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.
Confused on the logic here. Why can't we show both? one as HEAD
and one as LATEST
. That might be more align with what we do everywhere else? wydt
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.
seeing both is confusing don't you think? I want to keep SNAPSHOT
to something a little bit obscure as it's more meant for development
...tension-dsl-data-space/src/models/protocols/pure/DSLDataSpace_PureProtocolProcessorPlugin.ts
Show resolved
Hide resolved
@@ -92,6 +93,7 @@ export class DepotServerClient extends AbstractServerClient { | |||
classifierPath: string, | |||
options?: { | |||
search?: string | undefined; | |||
scope?: DepotScope | undefined; |
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 I agree with this scope input for the api.
We should be able to get both HEAD and latest, just one or none, when we query in scope it could be booleans
includeHead
includeLatest
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.
hm, I don't have a strong comeback for that, but this is how depot
APIs are kind of naturally set up. In the older infrastructure of depot
before it gets open-sourced, this is also the pattern of usage they follow
Summary
Closes #631
Also,
mode
strategy forQueryBuilderState
(similar strategy we adopted inEditorStoreMode
in studio: rework handling of multiple SDLC instances #642DataSpace
:How did you test this change?
I added screen recordings
Legend Query new look n feel 💪
Screen.Recording.2021-11-12.at.10.14.51.AM.mov
Studio mapping editor new look n feel 🎸
Screen.Recording.2021-11-12.at.10.15.28.AM.mov