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

Dashboard landing page #10003

Merged
merged 8 commits into from
Jan 25, 2017

Conversation

stacey-gammon
Copy link
Contributor

@stacey-gammon stacey-gammon commented Jan 23, 2017

Similar to #9605, implement for dashboards.

  • Columns are sortable
  • Breadcrumbs are links and removed 'New' and 'Open' top nav options.

Only thing missing from previous implementation is pagination.
screen shot 2017-01-23 at 11 06 34 am

screen shot 2017-01-23 at 10 47 26 am

screen shot 2017-01-23 at 10 47 42 am

Still TODO:
 - Add clickable breadcrumbs
 - Remove New and Open top nav options
improve tests
Copy link
Contributor

@cjcenizal cjcenizal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome!! This new UX feels so much better!

I had a few comments and noticed a couple bugs:

  1. The "Reporting" menu item in the top nav is visible on the Dashboard Listing page.
  2. We're showing "PromptForItems" when the user does a search and gets no matches. Instead, we should show "NoItems" in such a situation. "PromptForItems" should only be displayed if the user has zero Dashboards.

* will contain both the url for the given breadcrumb, as well as the breadcrumb the url
* was generated for.
*/
export function getBreadCrumbUrls(breadcrumbs, url) {
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 this file name should be bread_crumb_urls.js (plural) to match up with this function, right?


<div class="kuiPromptForItems__actions">
<a class="kuiButton kuiButton--primary kuiButton--iconText"
href="#/dashboard/create">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be formatted like this:

<a
  class="kuiButton kuiButton--primary kuiButton--iconText"
  href="#/dashboard/create"
>


<th class="kuiTableHeaderCell" ng-click="listingController.sortHits()">
Name
<i
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you change this to a span? It's more semantic.

@stacey-gammon
Copy link
Contributor Author

stacey-gammon commented Jan 24, 2017

Comments addressed. Separate PR out for the issue with reporting here.

@ppisljar
Copy link
Member

ppisljar commented Jan 24, 2017

This looks great !

Would it be possible to share more code with implementation for visualize ?

@stacey-gammon
Copy link
Contributor Author

jenkins test this

@stacey-gammon
Copy link
Contributor Author

@ppisljar Refactor so dashboard + visualize don't duplicate so much code? I would love to do this, and even mentioned this to @cjcenizal when we were first talking about it (management also uses a similar table).

Given timing situation for 5.3 though, I was postponing that as a clean up task. Want to get view/edit mode in for 5.3 but this has to go first. Thought the refactor would take too long and didn't want to delay. Happy to create a task and assign it to myself so I don't forget though to come back to it.

@stacey-gammon stacey-gammon added Feature:Dashboard Dashboard related features :Sharing labels Jan 24, 2017
Copy link
Contributor

@cjcenizal cjcenizal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ppisljar
Copy link
Member

selenium tests are failing ... apart from that it looks good to me

- rename bread_crumb_url => bread_crumb_urls
- fix reporting link in dash (separate PR)
- Use NoItems instead of PromptForItems when searching
- Style fixes
@stacey-gammon stacey-gammon merged commit 5ba6603 into elastic:master Jan 25, 2017
elastic-jasper added a commit that referenced this pull request Jan 25, 2017
Backports PR #10003

**Commit 1:**
Introduce dashboard landing page

Still TODO:
 - Add clickable breadcrumbs
 - Remove New and Open top nav options

* Original sha: 416e618
* Authored by Stacey Gammon <gammon@elastic.co> on 2017-01-23T14:40:14Z

**Commit 2:**
use clickable breadcrumbs instead of 'New' and 'Open' nav items

* Original sha: 271dd35
* Authored by Stacey Gammon <gammon@elastic.co> on 2017-01-23T15:47:59Z

**Commit 3:**
code cleanup

improve tests

* Original sha: c6354b1
* Authored by Stacey Gammon <gammon@elastic.co> on 2017-01-23T15:53:53Z

**Commit 4:**
Add new empty dashboard styling

* Original sha: 2641d8b
* Authored by Stacey Gammon <gammon@elastic.co> on 2017-01-23T20:35:06Z

**Commit 5:**
Fix selenium tests

* Original sha: a1b84ca
* Authored by Stacey Gammon <gammon@elastic.co> on 2017-01-23T21:50:37Z

**Commit 6:**
Address code review comments

- rename bread_crumb_url => bread_crumb_urls
- fix reporting link in dash (separate PR)
- Use NoItems instead of PromptForItems when searching
- Style fixes

* Original sha: fd857dc
* Authored by Stacey Gammon <gammon@elastic.co> on 2017-01-24T14:44:43Z

**Commit 7:**
Use a constant for the landing page url

* Original sha: fbdaa84
* Authored by Stacey Gammon <gammon@elastic.co> on 2017-01-24T20:20:07Z

**Commit 8:**
Fix tests

* Original sha: 33b4f24
* Authored by Stacey Gammon <gammon@elastic.co> on 2017-01-24T20:53:08Z
stacey-gammon added a commit to stacey-gammon/kibana that referenced this pull request Jan 25, 2017
* Introduce dashboard landing page

Still TODO:
 - Add clickable breadcrumbs
 - Remove New and Open top nav options

* use clickable breadcrumbs instead of 'New' and 'Open' nav items

* code cleanup

improve tests

* Add new empty dashboard styling

* Fix selenium tests

* Address code review comments

- rename bread_crumb_url => bread_crumb_urls
- fix reporting link in dash (separate PR)
- Use NoItems instead of PromptForItems when searching
- Style fixes

* Use a constant for the landing page url

* Fix tests
stacey-gammon added a commit to stacey-gammon/kibana that referenced this pull request Jan 25, 2017
* Introduce dashboard landing page

Still TODO:
 - Add clickable breadcrumbs
 - Remove New and Open top nav options

* use clickable breadcrumbs instead of 'New' and 'Open' nav items

* code cleanup

improve tests

* Add new empty dashboard styling

* Fix selenium tests

* Address code review comments

- rename bread_crumb_url => bread_crumb_urls
- fix reporting link in dash (separate PR)
- Use NoItems instead of PromptForItems when searching
- Style fixes

* Use a constant for the landing page url

* Fix tests

fix tests
stacey-gammon added a commit that referenced this pull request Jan 25, 2017
* Introduce dashboard landing page

Still TODO:
 - Add clickable breadcrumbs
 - Remove New and Open top nav options

* use clickable breadcrumbs instead of 'New' and 'Open' nav items

* code cleanup

improve tests

* Add new empty dashboard styling

* Fix selenium tests

* Address code review comments

- rename bread_crumb_url => bread_crumb_urls
- fix reporting link in dash (separate PR)
- Use NoItems instead of PromptForItems when searching
- Style fixes

* Use a constant for the landing page url

* Fix tests

fix tests
@stacey-gammon stacey-gammon deleted the dashboard-landing-page branch April 6, 2017 11:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Dashboard Dashboard related features review v5.3.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants