Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
plots: copy edit walkthrough and dashboard tooltips #1922
plots: copy edit walkthrough and dashboard tooltips #1922
Changes from 4 commits
49eec05
2b6b8f1
64b624d
4b641c8
51e4f48
abc0ff8
c6bed38
562bae5
5f4d273
a9bb31b
2151326
376b3d0
76a5b79
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
You are adding too much context to the page, too much structure. It tried as much as I can to keep it in a way that at least on my 13'' screen I can see some actual plots screenshots when open it
This comment was marked as resolved.
Sorry, something went wrong.
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.
Are you referring to the H2s further down? Also not a strong opinion, I guess it's a stylistic choice. IMO it looks better and clearer (
# Plots > ## Plot types
) and again it doesn't make the page longer. But I'm happy to flatten it if you confirm that's what we're talking about.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.
It's not about the page length per se. It's about when I can see any plots on the page that is called "Plots". We want the screenshot with them visible asap. Adding more paragraphs, structure, etc are making it more and more like a tutorial. We should be throwing out things if possible, not adding them. It's similar to DVC's get started - we are fighting for every symbol here.
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.
OK, I compressed it again, in fact I merged a few things to make it even shorter. PTAL
Maybe we should move the 💡 tip about setting up plots to the bottom then? Or just link to DVC docs
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.
Let's create a ticket please to being able to include links into these descriptions.
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.
Ticket created: #1933
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.
Do you also think "graphs" is also not good here? I like it better here since trends charts are not regular DVC plots but rather an extension-specific feature I think. Or does DVCLive also update regular plots in your browser? Anyway, feel free to also commit this if you prefer so:
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.
yes. Also not sure about making it the "Experiments Table" tbh.
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.
It was already "Experiments Table". That's the official name we give to that custom VS Code Editor I think?
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.
sorry, I meant capitalization - should it be "Experiments Table" or just "experiments table" in these descriptions?
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.
I like capitalization and bolding since each one has a walkthrough page. No strong opinion
BTW this page should be Plots Dashboard, I think (bit stronger opinion there).
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.
okay, let's keep it