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

Update adminLTE to 2.4.18 #1272

Merged
merged 7 commits into from May 12, 2020
Merged

Update adminLTE to 2.4.18 #1272

merged 7 commits into from May 12, 2020

Conversation

PromoFaux
Copy link
Member

@PromoFaux PromoFaux commented May 12, 2020

By submitting this pull request, I confirm the following:

  • 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, and have included unit tests where possible.
  • 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)

What does this PR aim to accomplish?:

  • Change a few references per upgrade guide
  • define colors[] at the top of index.js as AdminLTE.options does not appear to exist. (there may be a better way of doing this)

Alternative to #1271, which does the same thing but modifies an older version of AdminLTE

Both paths achieve the same thing.

All credit due to @roccivic who took the time to triage the responsiveness issue

 - Change a few references per upgrade guide
 - define colors[] at the top of index.js as AdminLTE.options does not appear to exist. (there may be a better way of doing this)

Signed-off-by: Adam Warner <me@adamwarner.co.uk>
@PromoFaux PromoFaux requested review from XhmikosR and DL6ER May 12, 2020 12:00
Signed-off-by: Adam Warner <me@adamwarner.co.uk>
@XhmikosR
Copy link
Contributor

XhmikosR commented May 12, 2020

I wonder maybe we could update to the latest 2.x version while at it?

Assuming you tested the changes, then the remaining diff is this: ColorlibHQ/AdminLTE@v2.4.12...v2.4.18

I cannot test this myself, but if it works for you guys go ahead. BTW feel free to squash my patch :)

DL6ER
DL6ER previously requested changes May 12, 2020
Copy link
Member

@DL6ER DL6ER left a comment

Choose a reason for hiding this comment

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

Something is strange here, look at the colors in this graph:
Screenshot at 2020-05-12 16-57-13

So I cleared my cache and reloaded the page and everything seemed to be back to normal:
Screenshot at 2020-05-12 16-57-25

However, when reloading (with or without flushing the cache), I can reproduce both variants here in a seemingly nondeterministic fashion.

edit

This also applies to the other doughnut, however, not the bar graphs, it seems.

Screenshot at 2020-05-12 17-00-02
Screenshot at 2020-05-12 17-00-15

edit 2

I see what is causing the issue and will push a fix in a minute.

@PromoFaux
Copy link
Member Author

I wonder maybe we could update to the latest 2.x version while at it?

Huh, I could have sworn I grabbed the latest 2.4.x version, perhaps not!

Something is strange here, look at the colors in this graph:

I took the colours from the documentation of AdminLTE <2.4 I may have got them wrong, of course!

@XhmikosR
Copy link
Contributor

It seems another class is missing from body too hold-transition BTW.

@DL6ER
Copy link
Member

DL6ER commented May 12, 2020

I fixed the color issue in 2314a60. @XhmikosR You missed the Sign-Off in your commit. Do you want to signal your agreement in a comment? We can then set DCO to pass manually.

Signed-off-by: Adam Warner <me@adamwarner.co.uk>
@XhmikosR
Copy link
Contributor

@DL6ER I don't mind, you even squash it with the other patches which I personally prefer :)

@DL6ER DL6ER dismissed their stale review May 12, 2020 15:22

Addressed the color issue

@PromoFaux PromoFaux changed the title Update adminLTE to 2.4.15 Update adminLTE to 2.4.18 May 12, 2020
@DL6ER
Copy link
Member

DL6ER commented May 12, 2020

@PromoFaux I see strange artifacts when scrolling with your last push. Something seems broken with 2.4.18.

git checkout 2314a60d
# Everything is fine

git checkout 9bf9e3b8
# I get lags when scrolling

I will try to record a GIF.

@PromoFaux
Copy link
Member Author

That's odd.. seems OK to me when scrolling. Did you clear cache? I will await your gif :)

@DL6ER
Copy link
Member

DL6ER commented May 12, 2020

Yes, I cleared the cache and force-refreshed.
ezgif-1-99bbfc73bf03

See the odd dark blocks when scrolling. They are absent one commit earlier (again cleared cache before trying that).

I'm using:
Screenshot at 2020-05-12 17-29-08

PromoFaux and others added 2 commits May 12, 2020 16:33
Signed-off-by: Adam Warner <me@adamwarner.co.uk>
@DL6ER
Copy link
Member

DL6ER commented May 12, 2020

Confirmed fixed after a Firefox update.

Screenshot at 2020-05-12 17-42-20

@DL6ER DL6ER merged commit 7eba926 into devel May 12, 2020
@DL6ER DL6ER deleted the tweak/updateAdminLTE branch May 12, 2020 17:52
@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.

None yet

4 participants