Skip to content

Create new Add Data landing page#5956

Merged
Bargs merged 7 commits intoelastic:feature/ingestfrom
Bargs:ingest/addDataStart
Jan 25, 2016
Merged

Create new Add Data landing page#5956
Bargs merged 7 commits intoelastic:feature/ingestfrom
Bargs:ingest/addDataStart

Conversation

@Bargs
Copy link
Copy Markdown
Contributor

@Bargs Bargs commented Jan 20, 2016

This PR makes the existing index pattern creation screen a subsection of the new Add Data landing page. This gives us a spot to link to the new ingest wizards allowing people to easily get data into elasticsearch if they don't have any yet.

Note that this PR is against feature/ingest, not master, so this isn't supposed to be the final polished version we'll ship, it's just concerned with re-organizing the pages.

@Bargs
Copy link
Copy Markdown
Contributor Author

Bargs commented Jan 20, 2016

Ah boy, forgot about functional tests, I'll assign back to myself until I fix them.

@Bargs Bargs assigned Bargs and unassigned jbudz Jan 20, 2016
@Bargs Bargs removed the review label Jan 20, 2016
@Bargs Bargs mentioned this pull request Jan 21, 2016
15 tasks
@Bargs Bargs added the review label Jan 22, 2016
@Bargs Bargs assigned jbudz and unassigned Bargs Jan 22, 2016
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

typo, editing?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Looks like it :) I just moved this existing code from index.js into its own file, I think I'll leave it alone to avoid accidentally breaking anything.

@jbudz
Copy link
Copy Markdown
Contributor

jbudz commented Jan 22, 2016

Code LGTM. Thoughts on:

  1. Noted above, indices/create/existing and indices/{{id}} may conflict if id === create. Routing should behave fine but it may be misleading
  2. The back button isn't catching indices/create and usually takes me back to discover.
  3. This is fine for now because there is only one option, but when more are added it might be useful to have the "Add new" button available on indices/create/existing and any new routes

@jbudz jbudz assigned Bargs and unassigned jbudz Jan 22, 2016
@Bargs
Copy link
Copy Markdown
Contributor Author

Bargs commented Jan 22, 2016

  1. I had the same concern, but I couldn't think of a good alternative that didn't look really weird in the context of the page structure. Would #/settings/indices_create/existing seem ok to you?

  2. Back button seems to behave a bit oddly in general in the settings app... I'll see if I can do anything about it though.

  3. Would the "Add new" button take you back to the landing page with the wizard options? Or do you mean something else?

@Bargs
Copy link
Copy Markdown
Contributor Author

Bargs commented Jan 22, 2016

Zoomed with Rashid and confirmed the back button is an existing issue throughout the settings app. Created a separate issue for it here #5982.

@Bargs Bargs mentioned this pull request Jan 23, 2016
@jbudz
Copy link
Copy Markdown
Contributor

jbudz commented Jan 25, 2016

  1. It helps. Thoughts on something like indices/id/{{id}}?
  2. 👍
  3. Yep, the request mostly stemmed from the back button issue, there's no way to get back to the wizard page without reloading. Consistency too, it is available when an index field list is loaded
    image

@Bargs
Copy link
Copy Markdown
Contributor Author

Bargs commented Jan 25, 2016

@jbudz Oooo, how about indices/edit/{{id}} since that page is essentially an index pattern editor?

@jbudz
Copy link
Copy Markdown
Contributor

jbudz commented Jan 25, 2016

@Bargs Good call, I like it

@Bargs Bargs assigned jbudz and unassigned Bargs Jan 25, 2016
@Bargs
Copy link
Copy Markdown
Contributor Author

Bargs commented Jan 25, 2016

@jbudz updated the edit index pattern route and added the Add New button to every page but the landing page itself.

@jbudz
Copy link
Copy Markdown
Contributor

jbudz commented Jan 25, 2016

Pending passing jenkins, LGTM.

@jbudz jbudz assigned Bargs and unassigned jbudz Jan 25, 2016
@Bargs
Copy link
Copy Markdown
Contributor Author

Bargs commented Jan 25, 2016

jenkins, test it

Bargs pushed a commit that referenced this pull request Jan 25, 2016
Create new Add Data landing page
@Bargs Bargs merged commit e05a3f9 into elastic:feature/ingest Jan 25, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants