-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Polish Create From Template Overlay #13424
Conversation
Use custom UI instead of responsive grid
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.
awesome!
const {createDashboardFromTemplate} = this.props | ||
|
||
await createDashboardFromTemplate(this.state | ||
.selectedTemplate as DashboardTemplate) |
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 would be much easier to read if they were two steps like:
dashboardTemplate = this.state.selectedTemplate as DashboardTemplate
await createDashboardFromTemplate(dashboardTemplate)
onSelectTemplate: (selectedTemplateSummary: TemplateSummary) => void | ||
} | ||
|
||
class DashboardTemplateBrowser extends PureComponent<Props> { |
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 could be an SFC?? :)
Columns, | ||
DapperScrollbars, | ||
} from '@influxdata/clockface' | ||
import {TemplateSummary, ITemplate} from '@influxdata/influx' |
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 think we should prefer to import from src/types
instead of directly from client since sometimes types are altered in the process.
let templateName = name || 'Untitled' | ||
|
||
if (version) { | ||
templateName = `${name || 'Untitled'} (v${version})` |
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.
hrmm. we are not currently using version in a way that makes it make sense to expose it so saliently to the user. I'm actually a little unsure how we should use versions. Maybe let's not display this to user. ?
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.
Sure! I figure it will be more useful in the future
Closes #13203
Closes #13343
DashboardCreateFromTemplateOverlay
Create
button if no template is selectedNotes
I think we could extract out some of these components into a generic set of
Create ___ from Template
components + styles.Checklist