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

IPC GUI fix bots collapse button #846

Closed
wants to merge 2 commits into from
Closed

IPC GUI fix bots collapse button #846

wants to merge 2 commits into from

Conversation

jaks97
Copy link
Contributor

@jaks97 jaks97 commented Jul 10, 2018

If I am not wrong, the boxBodyHTML content wasn't properly placed, causing the collapse button to not work properly (collapse worked but pressing the - wasn't doing anything).

@jaks97
Copy link
Contributor Author

jaks97 commented Jul 10, 2018

Also, I just hardcoded 'display: none;' to the box-body in my latest commit. This should fix the issue with animation not displaying the first time (yeah, it is not an AdminLTE bug).

@JustArchi
Copy link
Member

@MrBurrBurr Please review.

@JustArchi JustArchi added ✨ Enhancement Issues marked with this label indicate further enhancements to the program, such as new features. 👀 Evaluation Issues marked with this label are currently being evaluated if they're going to be considered. labels Jul 10, 2018
@MrBurrBurr
Copy link
Member

I will be home on the weekend, then I will review every PR :)
Thank you all for your patience.

@jaks97
Copy link
Contributor Author

jaks97 commented Jul 12, 2018

Fixing this made me see that when a bot is farming more than one game, the images are in a carrousel (wasn't working before). But that introduces a new bug, the first time you click the + all the images are displayed for a second and only then the carrousel is dispalyed:

This happens because the carrousel is initialized the first time that expanded.boxwidget event is triggered, and that event is only triggered only after the animation is fully completed.
One solution can be initializing all the carousels at the page load and then pause them. But if you did it this way probably you have your reasons (I think that maybe with houndreds of bots this will use a lot of uneeded resources at the page load).
Another solution could be make all the images of the games (except the first one) invisible by default (with display: none; or any other way). Then at expanded.boxwidget event, the carrousel is initialized and then all the images are visible again.

I don't know, what do you think @MrBurrBurr? I haven't tried any of my solutions tho, if I find free time soon I will check it better.

@MrBurrBurr
Copy link
Member

I am reviewing this PR right now and noticed some things.

  1. If I am not wrong, the boxBodyHTML content wasn't properly placed, causing the collapse button to not work properly (collapse worked but pressing the - wasn't doing anything).

It is placed correctly in the current version but this issue came with upgrading AdminLTE to version 2.3.4, whenever they added a fix for box inside box collapsing error. I went back to 2.3.3 and it works again.

  1. Also, I just hardcoded 'display: none;' to the box-body in my latest commit. This should fix the issue with animation not displaying the first time (yeah, it is not an AdminLTE bug).

First I want to state that this is in fact a bug in AdminLTE. Your fix will work in this case because how the box widget is used in IPC GUI (its hidden by default).


As you noticed yourself we now got the issue with the carrousel. I will have to think about what would be the best way to fix this since it wasnt coded by me but by @felipe19930.

Of couse it would be appreciated if you could test any of your solutions and give feed back :)

Thank you for your contribution.

@JustArchi

Please dont merge this PR. I added a part of this PR into #802.

@JustArchi
Copy link
Member

As per above. Thanks for review @MrBurrBurr

@JustArchi JustArchi closed this Jul 13, 2018
@MrBurrBurr MrBurrBurr mentioned this pull request Jul 20, 2018
@MrBurrBurr MrBurrBurr mentioned this pull request Jul 31, 2018
3 tasks
@lock lock bot locked as resolved and limited conversation to collaborators Aug 12, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
✨ Enhancement Issues marked with this label indicate further enhancements to the program, such as new features. 👀 Evaluation Issues marked with this label are currently being evaluated if they're going to be considered.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants