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

Fixes #1114: Implemented small description for each query #1115

Merged
merged 1 commit into from Jul 21, 2018

Conversation

4 participants
@praveenojha33
Copy link
Member

praveenojha33 commented Jul 20, 2018

Fixes #1114

Checklist

  • I have read the Contribution & Best practices Guide and my PR follows them.
  • My branch is up-to-date with the Upstream master branch.
  • I have added necessary documentation (if appropriate)
  • Added Surge preview link

Changes proposed in this pull request:

  • Implemented small description for each query similar to Market Leader Google. I have implemented it using Wikidata Open Source API.

image

@codecov

This comment has been minimized.

Copy link

codecov bot commented Jul 20, 2018

Codecov Report

Merging #1115 into development will decrease coverage by 0.38%.
The diff coverage is 40%.

Impacted file tree graph

@@               Coverage Diff               @@
##           development    #1115      +/-   ##
===============================================
- Coverage        55.75%   55.36%   -0.39%     
===============================================
  Files               49       49              
  Lines             1252     1277      +25     
  Branches           154      155       +1     
===============================================
+ Hits               698      707       +9     
- Misses             462      477      +15     
- Partials            92       93       +1
Impacted Files Coverage Δ
src/app/reducers/index.ts 93.54% <100%> (+0.21%) ⬆️
src/app/services/knowledgeapi.service.ts 55.35% <27.77%> (-10.56%) ⬇️
src/app/reducers/knowledge.ts 58.82% <33.33%> (-5.47%) ⬇️
src/app/actions/knowledge.ts 57.14% <50%> (-2.86%) ⬇️
src/app/infobox/infobox.component.ts 60% <55.55%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aac4376...83d897e. Read the comment docs.

@@ -55,6 +56,21 @@ export class KnowledgeapiService {
res.json()
).catch(this.handleError);
}
getQueryDescription(searchquery) {
let params = new URLSearchParams();

This comment has been minimized.

@simsausaurabh

simsausaurabh Jul 20, 2018

Member

Use const if the value is never assigned again.

params.set('search', searchquery);
params.set('language', 'en');
params.set('format', 'json');
let headers = new Headers({ 'Accept': 'application/json' });

This comment has been minimized.

@simsausaurabh

simsausaurabh Jul 20, 2018

Member

Use const if the value is never assigned again.

params.set('language', 'en');
params.set('format', 'json');
let headers = new Headers({ 'Accept': 'application/json' });
let options = new RequestOptions({ headers: headers, search: params });

This comment has been minimized.

@simsausaurabh

simsausaurabh Jul 20, 2018

Member

Use const if the value is never assigned again.

@simsausaurabh
Copy link
Member

simsausaurabh left a comment

@praveenojha33 The description exceeds the expected length and some part appears below the image.

screenshot from 2018-07-20 15-34-27

Please provide your view on it.

@praveenojha33 praveenojha33 force-pushed the praveenojha33:info branch from 2ead91d to 83d897e Jul 20, 2018

@praveenojha33

This comment has been minimized.

Copy link
Member

praveenojha33 commented Jul 20, 2018

@simsausaurabh I have made all suggested changes. Please review again.
image

@simsausaurabh
Copy link
Member

simsausaurabh left a comment

LGTM 👍

@mariobehling

This comment has been minimized.

Copy link
Member

mariobehling commented Jul 20, 2018

Looking at the picture of "Trump" do you see that you squeeze that image? With one fix somewhere another area is affected and a new issue appears. It does not make sense to work in that way - fix one thing and break another.

@praveenojha33

This comment has been minimized.

Copy link
Member

praveenojha33 commented Jul 21, 2018

@mariobehling I have not made any changes to image (just made some adjustment to position of image). But if you think there are issues with image (stretching or squeezing) I have just now opened an issue for the same #1116 .
Please compare the images
On susper.com
image

In this PR:
image

@mariobehling mariobehling merged commit 92ffeed into fossasia:development Jul 21, 2018

2 of 4 checks passed

codecov/patch 40% of diff hit (target 55.75%)
Details
codecov/project 55.36% (-0.39%) compared to aac4376
Details
Codacy/PR Quality Review Up to standards. A positive pull request.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@praveenojha33 praveenojha33 deleted the praveenojha33:info branch Jul 27, 2018

praveenojha33 added a commit that referenced this pull request Jul 28, 2018

Release 2: Merge development into master (#1135)
* Fixes #1107: Fixed CORS error by providing a check for empty query (#1108)

* updating pagination bar fixes #1104 (#1110)

* Fixes #1111 and #1112: Sentences do not terminate in infobox (#1113)

* Fixes #1114: Implemented small description for each query (#1115)

* Update README.md

* Fixes #1117: Fixed hardcoded URL (#1119)

* Fixes #1117: Fixed hardcoded URL

* Added field value pair for object.

* Fixes #1116: Fixed streching/squeezing of images (#1120)

* Fixes #1123: Fixed appearance of Intelligence box (#1126)

* Fixes #1121: Links and text length similar to Google (#1127)

* Fixes #1116 #1098: Fixed appearance of images for different browsers and different screen sizes. (#1128)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment