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

Removed search bar on 404 page #1452

Open
wants to merge 3 commits into
base: development
Choose a base branch
from

Conversation

samagragupta
Copy link
Member

@samagragupta samagragupta commented Feb 2, 2019

Fixes #1430

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:

-Removed search bar on 404 page

404

@samagragupta
Copy link
Member Author

samagragupta commented Feb 2, 2019

@Orbiter @mariobehling @simsausaurabh @praveenojha33 Please review

Copy link
Contributor

@subhahu123 subhahu123 left a comment

Choose a reason for hiding this comment

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

still this is not ready
please check spec.ts file there is still something to remove
@samagragupta
Also travis is failing because of that

@samagragupta
Copy link
Member Author

@subhahu123 Done the changes.

@codecov
Copy link

codecov bot commented Feb 3, 2019

Codecov Report

Merging #1452 into development will decrease coverage by 0.04%.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff               @@
##           development    #1452      +/-   ##
===============================================
- Coverage         62.2%   62.16%   -0.05%     
===============================================
  Files               51       51              
  Lines             1450     1443       -7     
  Branches           181      180       -1     
===============================================
- Hits               902      897       -5     
+ Misses             438      436       -2     
  Partials           110      110
Impacted Files Coverage Δ
src/app/not-found/not-found.component.ts 87.5% <ø> (+7.5%) ⬆️
src/app/services/intelligence.service.ts 75% <0%> (-4.17%) ⬇️
src/app/speechtotext/speechtotext.component.ts 42.85% <0%> (-2.86%) ⬇️
src/app/intelligence/intelligence.component.ts 76% <0%> (+12%) ⬆️

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 5221a57...450b55c. Read the comment docs.

subhahu123
subhahu123 approved these changes Feb 3, 2019
Copy link
Contributor

@subhahu123 subhahu123 left a comment

Choose a reason for hiding this comment

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

Remove autocomplete service also

@samagragupta
Copy link
Member Author

@subhahu123 Done.
@simsausaurabh @praveenojha33 Please review.

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.

@samagragupta I am unable to open test link for your PR.
image
Also I do not think leaving a lot of empty space on 404 will be good idea. It will be great if few other people @AakashMallik @shreyanshdwivedi can suggest their views on it. If you can provide a heroku link please provide that.

@samagragupta
Copy link
Member Author

@praveenojha33 The link is working for me.
Should I add some other image instead of that?

@praveenojha33
Copy link
Member

@samagragupta Please add a heroku link I am unable to navigate to 404 page.
image

@praveenojha33
Copy link
Member

@samagragupta Please provide heroku link I am unable to see changes on 404 page using surge link. The image is appearing odd to me as we do not have anything in lower half of the screen. If possible make it similar to google. Something like this http://google.co.in/dnsacxklsq. You can crop the current image and put it in middle.

@samagragupta
Copy link
Member Author

@samagragupta Please add a heroku link I am unable to navigate to 404 page.
image

This is what I have made changes.
I have removed the search bar.

@samagragupta
Copy link
Member Author

@mariobehling Please review.

@simsausaurabh
Copy link
Member

@samagragupta Could you please deploy it on heroku and provide link to review?

@simsausaurabh
Copy link
Member

@praveenojha33 Are we good to go?

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.

Search bar should be removed on 404 page
6 participants