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

Issues with new Listing component #31599

Closed
3 tasks done
timroes opened this issue Feb 20, 2019 · 5 comments
Closed
3 tasks done

Issues with new Listing component #31599

timroes opened this issue Feb 20, 2019 · 5 comments
Assignees
Labels
blocker bug Fixes for quality problems that affect the customer experience Feature:Visualizations Generic visualization features (in case no more specific feature label is available) Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.2.0

Comments

@timroes
Copy link
Contributor

timroes commented Feb 20, 2019

With #30610 a new listing component was introduced for visualization and dashboard listing. There are a few issues, we still need to fix before releasing:

  • The actual "link" has been moved from being a link (a) element to being a button with that PR. We should make sure that the link is actually a link for a couple of reasons: Better accessibility (navigation should be links, action should be buttons), with a button we prevent users from opening that page in a new tab.
  • The sort order on the dashboard page broke (and is now the same behavior than on the visualization page). We now always sort alphabetically. In case the user entered a query we don't want to sort alphabetically, but sort by the natural order of elements (i.e. the order that the API returned it to us), since that takes "better matches" into account. You can see that behavior in the SavedObjectFinder component. This was already broken for visualization beforehand (see Visualizations are always sorted alphabetically #14687) but with the new component we now also broke it on the dashboard listing page.
  • The edit button on dashboard does not longer jump to edit mode. Currently the edit button will only jump to the dashboard, but no longer (as beforehand) to the edit mode of the dashboard itself (see https://github.com/elastic/kibana/pull/30610/files#diff-77c2881567e7c051e799f0ccd0f3b60aL467 for the previous logic).

cc @lizozom @flash1293

@timroes timroes added bug Fixes for quality problems that affect the customer experience Feature:Visualizations Generic visualization features (in case no more specific feature label is available) Team:Visualizations Visualization editors, elastic-charts and infrastructure labels Feb 20, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app

@lizozom lizozom self-assigned this Feb 20, 2019
lizozom pushed a commit to lizozom/kibana that referenced this issue Feb 21, 2019
@lizozom
Copy link
Contributor

lizozom commented Feb 21, 2019

Both issues are fixed in the PR.

Since the button is still a button (EUI table doesn't support having actions with links), I had to pass in two functions from the Angular code (one is a click handler and the other returns the URL).

Also, EUI InMemoryTable supports loading in unordered state, but once the user sorted once, he can't go back to an unsorted state. This is accepted behavior according to @timroes

I have opened an EUI issue for this.

@timroes
Copy link
Contributor Author

timroes commented Feb 22, 2019

@lizozom I think we either need to give the EUI team some support on implementing that, or at lest should check where that could fit on their roadmap, since we need it do be done (and fixed our stuff) by 7.1, to not cause the sorting issue to cause a regression.

@timroes
Copy link
Contributor Author

timroes commented Feb 25, 2019

@lizozom Since on visualization the edit button and the link itself do exactly the same thing I was wondering if we might just want to pass hideWriteControls={true} to the component for visualization and not show any edit button at all? Thoughts about that?

@lizozom
Copy link
Contributor

lizozom commented Feb 25, 2019

Hey @timroes I think it's good to have it for the sake of UI consistency..

lizozom added a commit that referenced this issue Feb 25, 2019
* Fixing bugs from #31599
* Make listing item links instead of buttons
* restore dashboard edit button behavior
lizozom added a commit to lizozom/kibana that referenced this issue Feb 25, 2019
* Fixing bugs from elastic#31599
* Make listing item links instead of buttons
* restore dashboard edit button behavior
lizozom added a commit that referenced this issue Feb 26, 2019
* Fixing bugs from #31599
* Make listing item links instead of buttons
* restore dashboard edit button behavior
@lizozom lizozom closed this as completed Mar 6, 2019
@lizozom lizozom reopened this Mar 6, 2019
@lizozom lizozom closed this as completed Mar 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocker bug Fixes for quality problems that affect the customer experience Feature:Visualizations Generic visualization features (in case no more specific feature label is available) Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.2.0
Projects
None yet
Development

No branches or pull requests

3 participants