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

Fix queries Clear Filters button #1299

Merged
merged 2 commits into from
May 23, 2020
Merged

Fix queries Clear Filters button #1299

merged 2 commits into from
May 23, 2020

Conversation

XhmikosR
Copy link
Contributor

@XhmikosR XhmikosR commented May 16, 2020

Now it should look like the rest of the buttons

Signed-off-by: XhmikosR [email protected]

By submitting this pull request, I confirm the following: {please fill any appropriate checkboxes, e.g: [X]}

{Please ensure that your pull request is for the 'devel' branch!}

  • I have read and understood the contributors guide, as well as this entire template.
  • I have made only one major change in my proposed changes.
  • I have commented my proposed changes within the code.
  • I have tested my proposed changes.
  • I am willing to help maintain this change if there are issues with it later.
  • I give this submission freely and claim no ownership.
  • It is compatible with the EUPL 1.2 license
  • I have squashed any insignificant commits. (git rebase)
  • I have Signed Off all commits. (git commit --signoff)

@XhmikosR XhmikosR requested a review from DL6ER May 16, 2020 15:56
@PromoFaux
Copy link
Member

image

The button is now visible by default, which is interesting, especially as the hidden="true" is still set.

When applying a filter and then clicking the button it disappears as you would expect, but when reloading the page it comes back.

@XhmikosR
Copy link
Contributor Author

I don't see how this can can be related to this patch, though :/

@PromoFaux
Copy link
Member

Yeah, not sure, just commenting what I observed when checking out the branch :) Full cache reload between branches, too/

@XhmikosR
Copy link
Contributor Author

The exact same button classes are used everywhere else:

C:\Users\xmr\Desktop\AdminLTE>git grep -rna "id=\"resetButton\""
dns_records.php:84:                <button type="button" id="resetButton" hidden="true" class="btn btn-default btn-sm text-red">Clear Filters</button>
groups-adlists.php:68:                <button type="button" id="resetButton" hidden="true" class="btn btn-default btn-sm text-red">Reset sorting</button>
groups-clients.php:69:                <button type="button" id="resetButton" hidden="true" class="btn btn-default btn-sm text-red">Reset sorting</button>
groups-domains.php:130:                <button type="button" id="resetButton" hidden="true" class="btn btn-default btn-sm text-red">Reset sorting</button>
groups.php:66:                <button type="button" id="resetButton" hidden="true" class="btn btn-default btn-sm text-red">Reset sorting</button>
queries.php:150:            <button type="button" id="resetButton" hidden="true" class="btn btn-default btn-sm text-red">Clear Filters</button>

@XhmikosR
Copy link
Contributor Author

Let's leave this until we figure out the root cause.

@XhmikosR XhmikosR marked this pull request as draft May 17, 2020 11:28
@XhmikosR XhmikosR force-pushed the XhmikosR-patch-6 branch from c121435 to fc2826f Compare May 17, 2020 14:21
@XhmikosR
Copy link
Contributor Author

I confirmed the issue, although I have to say it doesn't make sense to me. I will try to fix it later by setting display: none or a class if we have one, and then it should be the same.

It's weird because it seems that only this button has this issue...

@XhmikosR XhmikosR force-pushed the XhmikosR-patch-6 branch 2 times, most recently from 620bb62 to 2218695 Compare May 18, 2020 08:04
@XhmikosR
Copy link
Contributor Author

@PromoFaux can you give it another go? BTW more of the reset buttons where showing on devel. This should take care of it.

@XhmikosR XhmikosR force-pushed the XhmikosR-patch-6 branch from 2218695 to 796a37d Compare May 18, 2020 08:13
@XhmikosR
Copy link
Contributor Author

So, apparently Bootstrap 3.x had https://getbootstrap.com/docs/3.4/css/#helper-classes-show-hide

I'll try to remove d-none later and make use of those. But probably in another PR.

Even I don't remember the 3.x utils... 🙁

Now it should look like the rest of the buttons

Signed-off-by: XhmikosR <[email protected]>
@XhmikosR XhmikosR force-pushed the XhmikosR-patch-6 branch 5 times, most recently from aa8ff24 to 064af1d Compare May 23, 2020 10:03
@XhmikosR XhmikosR marked this pull request as ready for review May 23, 2020 10:03
@XhmikosR XhmikosR force-pushed the XhmikosR-patch-6 branch from 064af1d to e70e108 Compare May 23, 2020 10:04
@PromoFaux PromoFaux merged commit b2aaf2b into devel May 23, 2020
@PromoFaux PromoFaux deleted the XhmikosR-patch-6 branch May 23, 2020 15:58
@XhmikosR XhmikosR added this to the v5.1 milestone May 29, 2020
@PromoFaux PromoFaux mentioned this pull request Jul 5, 2020
@pralor-bot
Copy link

This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there:

https://discourse.pi-hole.net/t/pi-hole-5-1-released/35577/1

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.

3 participants