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

Added "No PDFs found" #31

Closed
wants to merge 1 commit into from
Closed

Conversation

bradleyasu
Copy link
Contributor

Added a message that will be displayed if no PDF's were found. The message is a simple default li tag containing a div that will be removed if the count is greater than 0.

I think a search field will be a nice feature at some point. If it's added, it may be better to make the visibility hidden rather tan remove. That way, it can be made visible if the search yields no results.

Fixes issue

There is no issue for this - I was just kind of playing around with this. If you don't like the changes, please throw them out. I will make comments in the future about things and if you like the idea and there is a general issue open, I will commit to that.

Screenshots

nothingfound

before PR

after PR

Proposed Changes

If your current branch is master, you should choose to create a new branch for your commit and then create a pull request.

Added a message that will be displayed if no PDF's were found.  The message is a simple default li tag containing a div that will be removed if the count is greater than 0.

I think a search field will be a nice feature at some point.  If it's added, it may be better to make the visibility hidden rather tan remove.  That way, it can be made visible if the search yields no results.
@alexweininger
Copy link
Owner

I was thinking about this the other day! 👍👍 Looks perfect.

A search function and sorting are on my radar for the future.

@alexweininger
Copy link
Owner

Make sure you're on the most recent version of master

@bradleyasu
Copy link
Contributor Author

Sweet! - Yea, it should be. I'm not seeing any conflicts..

@alexweininger
Copy link
Owner

Hey @bradleyasu I'm sorry, but I've been having some weird glitches on your version. I'm not saying this was caused by you at all, but to make sure I am not going to merge it just yet.

I want some others to test out your version to confirm that it is good to go.

Here are the glitches, they might just be Chrome glitches. (sorry for my terrible circling)

image

@alexweininger alexweininger added bug Something isn't working enhancement New feature or request help wanted Extra attention is needed good first issue Good for newcomers hacktoberfest labels Oct 13, 2018
@alexweininger alexweininger mentioned this pull request Oct 13, 2018
@hwinn4
Copy link

hwinn4 commented Oct 14, 2018

@bradleyasu and @alexweininger Checked out this PR. I did not see the same UI issues, but I did notice a large gap below the footer. Anyone else seeing that? Here are some screenshots of how the extension renders in my browser:

screen shot 2018-10-14 at 5 15 37 pm
screen shot 2018-10-14 at 5 15 43 pm
screen shot 2018-10-14 at 5 16 45 pm

@alexweininger
Copy link
Owner

I am having the same issue @hwinn4. I'll have more time later this week to check it out, or @bradleyasu might fix it before I get to :)

@bradleyasu
Copy link
Contributor Author

Hey guys, sorry for being MIA on this lately - I apologize for the confusion and odd behavior, I haven't really experienced it, but will try to check it out this week as well.

@alexweininger
Copy link
Owner

@bradleyasu no problem at all! There's no rush.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Projects
update ui
  
Awaiting triage
Development

Successfully merging this pull request may close these issues.

None yet

3 participants