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

Doc viewer app #2121

Merged
merged 13 commits into from
Dec 5, 2014
Merged

Doc viewer app #2121

merged 13 commits into from
Dec 5, 2014

Conversation

rashidkpc
Copy link
Contributor

This allows users to open and share a link to exactly one document. Closes #717

screen shot 2014-12-04 at 4 26 24 pm


var computedFields = $scope.indexPattern.getComputedFields();

es.search({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Q: Why a search here?
A: Elasticsearch GET API doesn't support stored or scripted fields, have to go through the search API

Q: Well then why didn't you use a searchSource?
A: It seemed wasteful to start up a temporary indexPattern and searchSource for one request. I would need to new up an indexPattern as the existing one could match some range of indices, and I need exactly one index. The indexPattern that does get loaded here is only used for formatters and scripted fields retrieval.

@rashidkpc rashidkpc added this to the 4.0.0-BETA3 milestone Dec 5, 2014
@@ -0,0 +1,13 @@
define(function (require, module, exports) {
require('plugins/doc/controllers/doc');
require('css!plugins/doc/styles/main.css');
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't actually being compiled. Also, there doesn't appear to be any rules in plugins/doc/styles/main.less.

@w33ble
Copy link
Contributor

w33ble commented Dec 5, 2014

There's a weird style issue. I fixed it in rashidkpc#1

screenshot 2014-12-05 11 20 33

@w33ble
Copy link
Contributor

w33ble commented Dec 5, 2014

_get_computed_fields.js seems odd, since it's just fetching all date fields. But, I guess that's the point, and I'll probably add some more to this as part of the scripted fields stuff anyway.

So, remove that unused LESS file, and merge/modify that PR I sent you, and this LGTM

@w33ble w33ble removed the review label Dec 5, 2014
@w33ble w33ble assigned rashidkpc and unassigned w33ble Dec 5, 2014
@rashidkpc rashidkpc assigned w33ble and unassigned rashidkpc Dec 5, 2014
@w33ble
Copy link
Contributor

w33ble commented Dec 5, 2014

🍐

w33ble added a commit that referenced this pull request Dec 5, 2014
@w33ble w33ble merged commit b668145 into elastic:master Dec 5, 2014
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.

Link to a specific document
2 participants