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

Polish Create From Template Overlay #13424

Merged
merged 14 commits into from
Apr 16, 2019

Conversation

alexpaxton
Copy link
Contributor

@alexpaxton alexpaxton commented Apr 16, 2019

Closes #13203
Closes #13343

Screen Shot 2019-04-15 at 6 24 40 PM

Screen Shot 2019-04-15 at 4 04 25 PM

  • Rename component to DashboardCreateFromTemplateOverlay
  • Change title of overlay to match menu item that triggers it
  • Break component into a set of smaller components
  • Allow scrolling in both templates list and details panel
  • Show empty state that links to settings page when no templates exist
  • Display template version number next to the name
  • Show empty states when a template has no:
    • Name
    • Description
    • Variables
    • Cells
  • Display templates in a list rather than a grid for text legibility purposes
  • Polish styles for template details
  • Disable Create button if no template is selected

Notes

I think we could extract out some of these components into a generic set of Create ___ from Template components + styles.

Checklist

  • CHANGELOG.md updated with a link to the PR (not the Issue)
  • Rebased/mergeable
  • Tests pass
  • http/swagger.yml updated (if modified Go structs or API)
  • Sign CLA (if not already signed)

Copy link
Contributor

@ebb-tide ebb-tide left a 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)
Copy link
Contributor

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> {
Copy link
Contributor

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'
Copy link
Contributor

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})`
Copy link
Contributor

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. ?

Copy link
Contributor Author

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

@alexpaxton alexpaxton merged commit 2b8c17b into master Apr 16, 2019
@alexpaxton alexpaxton deleted the polish/create-from-template-overlay branch April 16, 2019 20:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Polish Import Dashboard From Template UI Import Dashboard From Template Overlay fixes
2 participants