Afterburner horizontal scroll in report

Thanks for reporting that.

Have you tried this in another theme? It would be good to know if the problem is isolated to this theme or if it is a problem with the quiz report itself.

Hello, I've tried all themes and the problem is is isolated to this theme "Afterburner"

I have tried different themes and have run into the same issue. You can look at http://moodle.org/mod/forum/discuss.php?d=195180 for more information.

The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

TIA and ciao

I am on the latest 2.1.4+ (from last Saturday), and the issue affects many core themes. It is not just Afterburner.

Brick, Fusion, Magazine, Nonzero, Nimble and Overlay have the scrollbar.

Anomaly, Standard, Boxxie, Leatherbound, Binarius, Arialist, Sky High and Splash do not.

I have some contributed themes installed as well, but I did not check those. I may not have the latest release.

Hi Rob,

Thanks for testing those themes. Looking at this it is probably something that could be done in the Mod itself. I'll have to investigate this.

Cheers
Mary

I've just halted the integration of this for the time being while I do some more testing.

Ok...I've made some changed to the original patch for this issue, so the bottom scroll bar, in the browser, scrolls as far as needed for any report page. This is far better than the previous fix. The beauty of this method is that you have the scroll bar available to you, at whatever depth of page you are at, you don't need to go hunting for it, as you would have had to do in my previous fix.

As for the other themes I might add them to this as sub-tasks...but not sure yet as there is quite a bit of work still ongoing with pagelayout-report in general in another tracker issue.

That's it done now...so all we can do now is wait.

Cheers
Mary

Hey mary.

the latest fix didnt work for me (Hebrew, RTL),
while the first fix

.pagelayout-report #page-content #region-main-box #region-post-box #region-main-wrap #region-main-pad #region-main .region-content .no-overflow

wokres perfectly !

I bet this must go back to 22 and 21 stables too, correct?

Also... about omer's comment above ... should we halt this until proved to work RTL?

Ciao

PS: Note to integrators... this contains 4-5 commits that ideally should be joined into one.

Eloy, I haven't quite got to grips with GIT so I am not sure how I should approach combining all the commits my changes to this patch created. I need to go to back to school to learn all this.

I'm not totally convinced about the RTL as the patch adds a scroll bar to the page not just to the table as Omar prefers.

Sam was not too keen on the first method, that's one of the reasons I dropped it in favour of the one which other theme use.

So this said... I would prefer this to go through now and if possible cherry-pick to 22 and 21 stable please. If you would prefer me to redo this as one commit I can.
Cheers

I have just re-worked this and cleaned up the commits, and also added fix for RTL in rtl.css which should fix the RTL scroll problem too.

Just so you know I know, after making sure all was saved in correct formal (UNIX UTF-8) I got a warning that LF would now become CRLF is this going to cause me a problem, Eloy?

Hi Mary, to combine commits you can try a 'git rebase -i' which is an interactive rebase. You should be prompted with an editor with some comments about how to proceed.

Generally, i've had to pick 's' for squashing commits, and then the specified commits would be combined (squashed) and a whole new commit message could be written to describe them all

Hi Aparup,

I have just redone them and added stable branches too and cherry picking along the way!

Hey, you're becoming a git-pro! squashing, cherry-picking... congrats! :-P

About to integrate this now...

Re: About cr/lf... I don't know why/where you did get that message, but it seems that your patch contains correct lf ones, so no problem. Which OS and environment (IDE) do you use for coding all this stuff?

Integrated (21, 22 and master), thanks!

Thanks Eloy!

As for OS & IDE (whatever that means)
Windows 7
GIT
TextPad (PHP & CSS set to UNIX UTF-8)

http://www.textpad.com/products/textpad/index.html

I got the message in GIT Bash when I came to do

git add file

prior to doing the commit.

Hi Mary,

Is your fix only for Afterburner, or will the other themes be fixed too?

Rob

Hi Rob,
This is an Afterburner fix ONLY.

Are other themes affected? I thought they were OK...if not then please can you report the themes affected in a new Moodle Tracker issue?

Thanks
Mary

Below is the list from my previous comment in this thread:

Brick, Fusion, Magazine, Nonzero, Nimble and Overlay have the scrollbar.

Anomaly, Standard, Boxxie, Leatherbound, Binarius, Arialist, Sky High and Splash do not.

We are using Sky High. My teachers use that report extensivly. There was a scrollbar in the past, but it went away in a recent update. In addition, we allow course themes, so fixing the others would be helpful too. Thank you for all your help.

Rob

Ah...sorry I forget which day it is let alone which bug I am working on! LOL

I have this now...and will probably get this sorted out for next week's pull.

I have just created MDL-31566 in an attempt to fix this in other CORE themes too.

Hi Mary,
I tried to test this, but the column still appears chopped off for me. This can be due to other window specific warnings(Already existing issue -invalid CRT parameters) messing with the page layout on my end. I will wait till someone on lamp takes this up and gives it a try.
Thanks

Hi Ankit,

Any progress with this?
All the fix does is adds a scroll bar to the bottom of the browser window, and if you use a RTL language the actual table on the page gets a scroll bar too (or should!)

Cheers
Mary

Hi,

I'm attaching one screen-shot of how the table and the window looks after applying the patch, exactly the same happens both with Safari and Firefox here:

1) The table continues not showing any scroll-bar, so contents are cut.

2) The window has one horizontal scroll-bar, but using it only leads to blank contents. Only making the window BIG (1900) I can see both the table and the graph properly.

So I'm going to revert and reopen this issue. Clearly I'm not getting the scroll bar in the table as expected.

Ciao

Hi Mary,

in my eyes we only have to add the following line to afterburner-layout.css for achieving the horizontal scrollbar in the right way:

.pagelayout-report #report-page-content #report-region-main-box #report-region-post-box #report-region-main-wrap #report-region-main .region-content

[tested with IE8/9, Firefox and Chrome + ltr/rtl].

"My" IE7 seems to be unable to handle afterburner in combination with rtl languages!?

Cheers
Dietmar

Test with Safari 5.1.2 and rtl/ltr: ok

Hi Dietmar,

Afterburner doesn't use the report page layout, it uses default.php.
I'm just wondering if you are testing Sky High as we have been working on that too, and there is a horizontal scroll problem on that theme also. So if this is the report of your testing Sky High that's great! Thanks

I am still struggling with Afterburner as it worked well when I tested it last week now it's all over the place!

This is what I had submitted which has been rejected, as it was not working the way I thought it would.

.pagelayout-report #page-content {float:none; width:auto;}

.pagelayout-report #page-content,
.pagelayout-report #page-content #region-main-box,
.pagelayout-report #page-content #region-main-box #region-post-box,
.pagelayout-report #page-content #region-main-box #region-post-box #region-main-wrap #region-main-pad,
.pagelayout-report #page-content #region-main-box #region-post-box #region-main-wrap #region-main-pad #region-main {overflow:visible;}

I finally got it back to...

.pagelayout-report #page-content #region-main-box #region-post-box #region-main-wrap #region-main-pad #region-main .region-content .no-overflow {overflow: auto;}

which works OK in...
FF (v.10)
Opera (11.52)
Chrome (v.16.0.912.77 m)
IE9

@Aparup

I tried out the git rebase -i and got into all kinds of problems.
I am still wondering where VIM editor came from...what an antique of a thing that is! I couldn't quite grasp what I was supposed to do, but I did find some GIT documentation which explains it.

But thanks just the same

@Mary: I'll test that with a clean theme installation and be back this afternoon!

@Mary: afterburner pagelayout for reports works fine now!

Will have a look at sky_high later!

Cheers
Dietmar

Hi Mary
sorry you ran into such trouble with vim, it can be really painful. Perhaps with textpad and windows you can look at how this guy made Git use textpad instead of vim with one setting.

The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

TIA and ciao

@Aparup

You are a STAR! Thanks for that...works great! I can do commit -a yeah! I never quite knew what that did till now! saves lode of time LOL
I must try the squash one now I know what I am doing!

Thanks

Integrated thanks!

Note: Why is that theme soooo strange that it never shows the window horizontal scroll bar? Any page you try, for example, course page, if reduced horizontally looks really horrible.

Tested under safari, chrome, firefox (all OS X). The table now shows the horizontal scroll bar if necessary.

But, as commented above, other contents aren't shown properly as far as the windows never shows the scrollbar. For example the course page, or, in the same quiz report, the graph is wider and you only can get it by making the window wide enough. For your consideration.

Anyway, this passes. Ciao

Some changes to Moodle should be milestones in the project by themselves.

This is not the case and your fix is not so important, but your collaboration is highly appreciated, thanks!

Closing as fixed, ciao