-
Notifications
You must be signed in to change notification settings - Fork 5
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
Revert legend explicit positioning since it breaks VS Code #115
Comments
Alternative from iterative/vscode-dvc#3226 => VS Code walks the templates and inserts the correct information into the templates at the correct place instead of applying overrides at the bottom of the template's encoding. |
Agreed, I think we can traverse and drop the legend objects as we do with titles. It should be safe I think. Let me try do this first. |
Since the main rationale of #113 was to improve the UI in Studio and VS Code, should we still consider reverting the change? Is it likely to always be important to have custom legend handling in those products, and does that change make it harder? |
@dberenbaum we can remove legends programmatically (see iterative/vscode-dvc#3228) so we don't need to revert the change. However, take a look at this PR: iterative/vscode-dvc#3229 I tried moving the legend to the top for the zoomed plots behaviour. When the User selects the max number of revisions (7) this gets pretty unwieldy. I think we will be keeping our legends on the right for now. How about we chat about this one in our next cross-team meeting. |
I'd still vote to revert for consistency across products. If the legend is on the right in VS Code, let's do it everywhere unless we have a good reason. Removing the legends programmatically also just feels like adding more places things can break, so why not revert and save the trouble? |
Up to you. I think VS Code will still need to programmatically remove legends. This change has demonstrated that custom templates are definitely going to break the VS Code UI. |
Since we don't prioritize custom templates for now, I think we should be good. @dberenbaum your call on this. I can close the PR then. |
I suggest we revert #113. Plots have become unmanageable and I can't find a quick fix for this:
If there are more rev selected plot becomes smaller and smaller.
Here is the Vega config with the VS Code customizations (including an explicit instruction to disable the legend, but it's not working in this case):
The text was updated successfully, but these errors were encountered: