oupala / apaxy

a simple, customisable theme for your apache directory listing
https://oupala.github.io/apaxy/
GNU General Public License v3.0
1.86k stars 256 forks source link

Add a Search Bar #64

Closed woodrowbarlow closed 5 years ago

woodrowbarlow commented 8 years ago

This adds a search bar at the top which filters the results on the current page. Only filters by name. Only filters what's on the current page (no searching within sub-directories).

Looks like this:

filter

The box expands to this (with a short animation) when focused:

filter2

woodrowbarlow commented 8 years ago

i also have code that implements the following features:

also working on automatic pagination for large tables.

i'm waiting on your feedback to submit these, in case you have code quality comments. in particular, i noticed that you avoid using classes anywhere in your CSS, but i've used a few -- wondering why you avoided them and whether you're fine with my code using them.

hazcod commented 8 years ago

I would love to see this merged, @Adam

samfield1 commented 8 years ago

This is great except I experienced a few problems when using it.

Unable to Hide Rows

The row hider seemed to incorrectly reference the row textContent with getElementsByClassName('indexcolname') in lines 39 and 50 in footer.html I hackishly changed this to getElementsByTagName('a')

Hiding Name - Last Modified - Size Row

No Filter image Filtering for mastermind image

As you can see the table header was set not visible, ruining the row styling. I 'fixed' this by sticking if (row.getElementsByTagName('a')[0].textContent === "Name" && row.getElementsByTagName('a')[1].textContent === "Last modified" && row.getElementsByTagName('a')[2].textContent === "Size") return; at the beginning of the _filter() function. Obviously it would be better if the _filter() never iterated over this in the first place, but after some time fiddling with the Arr.forEach.call()s in _onInputEvent() I could not get it to work.

Obviously I am not knowledgeable of the workings of JS and this commit to properly fix it. Please consider good fixes for these issues I have found.

woodrowbarlow commented 8 years ago

Hello @feblehober123,

I'm afraid I don't see the issue with lines 39 and 50.

var text = row.getElementsByClassName('indexcolname')[0].textContent.toLowerCase();
var secondRowName = secondRow.getElementsByClassName('indexcolname')[0].textContent;

The HTML generated from my version of Apache in a directory listing contains classes on each row of the table, like so. Does your generated HTML not contain these classes?

screenshot from 2016-06-21 15-51-50

I am using Apache/2.4.7 (Ubuntu). Filtering works for me using the code exactly as I've submitted here.

screenshot from 2016-06-21 15-57-59

samfield1 commented 8 years ago

Nevermind then, it must be a problem just with my older version Apache/2.2.31 (Gentoo). Thank you for looking into it regardless.

woodrowbarlow commented 8 years ago

@feblehober123 when i have a second, i'll fix my PR to avoid using class names. there's no reason it can't support older versions of apache.

woodrowbarlow commented 8 years ago

@feblehober123 i've re-written the filtering code -- it should work for you now.

i'd actually appreciate if you'd test it out for me, since my version of apache isn't like yours.

samfield1 commented 8 years ago

@woodrowbarlow It works fine with my version now. Thanks for looking into it!

woodrowbarlow commented 6 years ago

@AdamWhitcroft can this be merged?

oupala commented 6 years ago

This branch has conflicts that must be resolved.

apaxy has been transferred to a new team and we're trying to do the housework between all pending issues and pull requests.

Is it possible for you to fix conflicts so we can test this branch and decide what to do with it?

woodrowbarlow commented 6 years ago

@oupala that should be it.

i don't currently have an apache server running, so please test this before merging.

oupala commented 6 years ago

This look good.

Could you please follow conventional changelog convention for your commit messages? It allows to generate an automatic changelog file.

Please use angular convention.

oupala commented 6 years ago

This pull request adds an interesting feature. I tested it and it works well.

Can anyone else review this pull request and tell me about it? If someone else has a positive point of view on it, I would want to merge it.

ghost commented 6 years ago

I tried it. It works flawlessly. 👍

chrissy-dev commented 6 years ago

@oupala @jordanbancino Can we wait until #103 is merged and I'll work it into the design of Apaxy?

oupala commented 6 years ago

Ok. But I did not understood what you wanted to work on.

woodrowbarlow commented 5 years ago

this says merged but if i look at apaxy.js at the tip of master, the filter code isn't there.

oupala commented 5 years ago

You're right. I merged the branch until I realized that the merge was toward the master branch.

As we have a develop branch which is devoted to develop commits, and to testing, I rebased the branch to a new branch and deleted the commits from the master branch.

I'm gonna test it a little bit, and merge it to the develop branch once it is ok.

oupala commented 5 years ago

It seems to work well. Can you please test the feature/search-bar branch and tell me if you think its ok?

woodrowbarlow commented 5 years ago

i'm no longer using apache on my personal server, so i don't have an environment to test in. the code looks like it's in a good state to me. please merge at your discretion. i'm glad to have contributed!

PS: if you choose to re-write history at all when integrating this to the develop branch, i would appreciate if you did so in a manner which keeps me listed as the commit author. :) i see that is already the case on the feature branch and i appreciate that.

oupala commented 5 years ago

Thanks @woodrowbarlow for your answer.

As far as I have tested, it works pretty well! I just want to have a confirmation about commit 19c3def: do you think it is normal that 12 lines are deleted in this commit? Do all the javascript you added have at least the same features that was provided by the previous javascript?

And by the way, rebasing does not imply taking over your commits, and this is not my intention. I will carefully watch that you remain the author of these commits.

woodrowbarlow commented 5 years ago

oh, you're right, something does look wrong there.

the following lines got deleted and are not provided by my new code:

// fix links when not adding a / at the end of the URI
var uri = window.location.pathname.substr(1);
if (uri.substring(uri.length - 1) !== '/') {
  var indexes = document.getElementsByClassName('indexcolname');
  var i = indexes.length;
  while (i) {
    var a = indexes[i].getElementsByTagName('a')[0];
    a.href = '/' + a.getAttribute('href', 2);
  }
}
oupala commented 5 years ago

I've restored the deleted lines. I tested the updated branch and it works, as far as I can tell.

Can you confirm it?

If everything is ok, I'm gonna merge, at last! (You're pull request is 2 years and a half old!)

woodrowbarlow commented 5 years ago

here's the final diff, there's just one thing.

when you added the code block i mentioned above, you also added the two lines above it:

// grab the 2nd child and add the parent class. tr:nth-child(2)
document.getElementsByTagName('tr')[1].className = 'parent';

these two lines were removed on purpose because i moved them into my filter code:

if (row !== null && row.getElementsByTagName('td')[1].textContent === 'Parent Directory') {
    row.className += ' parent';
}

at the time, moving that line into my filter code made sense because that line was the only javascript in the whole project. the block i mentioned in my previous comment didn't exist when i opened this PR.

my code also makes an extra check to be sure there really is a link for "Parent Directory" before applying the class -- this is important because if you are running apaxy at your document root, there won't be a link for "Parent Directory".

oupala commented 5 years ago

I fixed the branch.

Does it look good to you now?

woodrowbarlow commented 5 years ago

looks great! i approve the merge.

oupala commented 5 years ago

I merged the branch to develop.

Thanks for the contribution @woodrowbarlow!