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

Fixes #188, #333, #543: Fix missing sidebar and improve results #754

Merged
merged 1 commit into from Jun 17, 2018

Conversation

simsausaurabh
Copy link
Member

@simsausaurabh simsausaurabh commented Jun 16, 2018

Changes proposed in this pull request

  • Created new source for sidebar results. The older config was not getting results.
  • Removed Trending hashtags from results page.
  • Added more parameters to api/search.json to provide filtered search.
  • Added source to be apiResponseResults from parent feed.component.ts to child feed-info.component.ts.

Screenshots (if appropriate)
The changes have been made and it looks more appropriate in live test link.

screenshot from 2018-06-17 01-15-52

screenshot from 2018-06-17 01-16-16

Link to live demo: http://pr-754-fossasia-loklaksearch.surge.sh

Closes #188
Closes #333
Closes #543

Note: Work is being carried out currently to show images on sidebar (The older implementation was trying to fetch images from twitter which do not works due to twitter updates).

Copy link
Member

@praveenojha33 praveenojha33 left a comment

Choose a reason for hiding this comment

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

There are some problems with Travis Please check, make sure to include your changes in test files.

@simsausaurabh simsausaurabh force-pushed the sidebar branch 3 times, most recently from 90344cb to eb83f08 Compare June 16, 2018 20:21
@codecov
Copy link

codecov bot commented Jun 16, 2018

Codecov Report

Merging #754 into development will decrease coverage by 0.75%.
The diff coverage is 5.26%.

Impacted file tree graph

@@               Coverage Diff               @@
##           development     #754      +/-   ##
===============================================
- Coverage        52.57%   51.82%   -0.76%     
===============================================
  Files               99       99              
  Lines             2488     2499      +11     
  Branches           272      276       +4     
===============================================
- Hits              1308     1295      -13     
- Misses            1154     1178      +24     
  Partials            26       26
Impacted Files Coverage Δ
src/app/reducers/index.ts 95.37% <ø> (ø) ⬆️
src/app/feed/feed.component.ts 31.32% <ø> (-13.12%) ⬇️
src/app/feed/info-box/info-box.component.ts 20.38% <5.26%> (-3.15%) ⬇️

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 2559bbd...2604e3d. Read the comment docs.

@simsausaurabh
Copy link
Member Author

simsausaurabh commented Jun 16, 2018

@praveenojha33 I will be writing test case for this patch and news tab collectively, since the changes will be made in almost same files so it will create chaos and bugs to manage it again. I will be writing clean tests combined for the sidebar feature and news tab feature. The changes are also a lot in the current patch.

@mariobehling
Copy link
Member

Thank you. Please learn how to make screenshots of web pages that are larger than your screen. It is very easy! There are also plugins available.

@mariobehling mariobehling merged commit 0d691dc into fossasia:development Jun 17, 2018
@simsausaurabh simsausaurabh deleted the sidebar branch June 17, 2018 04:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants