Skip to content

Fixing issue #2090 Searching night mode #2095

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

Closed
wants to merge 1 commit into from

Conversation

pereiraJr
Copy link

@pereiraJr pereiraJr commented Dec 9, 2018

Searching for images or categories in night mode works but the words doesn't show neither the recent researches.

Fixes #2090 Searching on Night Mode

What changes did you make and why?
Ux changes to be more clean on night mode

Tests performed (required)
Exploratory testing

Screenshots showing what changed (optional - for UI changes)

image

@codecov-io
Copy link

Codecov Report

Merging #2095 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #2095   +/-   ##
======================================
  Coverage    4.02%   4.02%           
======================================
  Files         226     226           
  Lines       11467   11467           
  Branches     1064    1064           
======================================
  Hits          461     461           
  Misses      10972   10972           
  Partials       34      34

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 677e009...2420127. Read the comment docs.

@neslihanturan
Copy link
Collaborator

Thanks for your first time contribution @pereiraJr :) I have made some notes, please consider them. By the way it can be nice adding before after UI comparisons for such UI changes
Basically before:
master

And after:
branch

There is a nice UI improvement. But you changed some of files you shouldn't. Whenever you fixed them we can review your PR again.

@neslihanturan neslihanturan self-requested a review December 17, 2018 12:24
@neslihanturan
Copy link
Collaborator

As final decision, I request changes @pereiraJr :)

@domdomegg domdomegg mentioned this pull request Dec 20, 2018
@domdomegg
Copy link
Member

After merging into master it doesn't seem to work for me:

Expected Actual
screenshot_1545302618 screenshot_1545306713

Also as I was trying to get this to work I've made some improvements to the search UI (see #2189), which might have an impact on how this is implemented. Might make it easier actually, as both can have the same toolbar colours. Will handle merge conflicts if they happen if that is merged first, please tag me if needed.

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.

4 participants