reposition footer #2269

Alex-Jordan · 2023-12-02T21:33:27Z

This puts the footer in a bootstrap row, and makes it a col-10 above the large breakpoint.

On large screens, the footer was looking too far to the right, imo. This generally nudges it a bit to the left.

drgrice1

I am okay with this. However, I do notice that on some pages where the content is not also restricted to the col-10 width (like the "User Manager", the "Set Manager", etc.), things look a little unbalanced. The footer is to the left of center relative to the full width content directly above it.

drgrice1 · 2023-12-02T22:30:25Z

htdocs/themes/math4/system.html.ep

@@ -172,7 +172,7 @@
 		% }
 		%
 		% # Footer
-		<div id="footer" role="contentinfo"><%= include 'ContentGenerator/Base/footer' %></div>
+		<div id="footer" role="contentinfo" class="row"><%= include 'ContentGenerator/Base/footer' %></div>


When I converted the footer into an included Mojolicious template from the old template system in which this was a template escape, I maintained the structure which had the containing div here. I really should have moved that containing div into the included footer template though. This is made worse conceptually with this change though, because this div has the row class which is important for the col class on the div in the included template. So I suggest that this be changed to

Suggested change

<div id="footer" role="contentinfo" class="row"><%= include 'ContentGenerator/Base/footer' %></div>

<%= include 'ContentGenerator/Base/footer' =%>

and the <div id="footer" role="contentinfo" class="row">...</div> be moved into the footer.html.ep file.

Alex-Jordan · 2023-12-02T23:24:18Z

The footer is to the left of center relative to the full width content directly above it.

Yeah, I'm noticing that now too on the login screen, which I really don't like since that's a screen everyone sees a lot. I'll rethink this.

Alex-Jordan · 2023-12-03T19:00:30Z

Is it possible for a template (like the main template for a page) to store a value in some way so the footer template can access it? Then the main templates could store something about their width (e.g. col-10) and the footer template could access that.

drgrice1 · 2023-12-03T21:32:02Z

Is it possible for a template (like the main template for a page) to store a value in some way so the footer template can access it? Then the main templates could store something about their width (e.g. col-10) and the footer template could access that.

Yes. I added a pull request to this branch that implements the mechanism for this. It is not complete though. I left the work of going through page by page and determining what width is needed to you. I think most pages will not need anything as the default of col-12 will work. You just need to figure out what pages need something different.

drgrice1 · 2023-12-03T22:13:42Z

I think that in most cases the footerWidthClass should be set in the templates in the same place where the primary layout is determined, and not in the .pm files. I just pointed out that it could be done there in case a situation is encountered where that makes more sense.

Alex-Jordan · 2023-12-03T23:10:38Z

I set the overall footer class in ContentGenerator.pm to match the conditional page body class from math4/system.html.ep, namely $c->can('info') ? 'col-md-8' : 'col-12'.

Then set the class in several pages where that the overall positioning was different.

In the Set Assigner template, the instruction text at the bottom is now in a bs grid that has the same width as the scrolling record lists above, and then of course the footer is the same.

In the scoring template, I changed to use a bs grid around the tools at the top of the page for selecting sets, and then made the footer match that.

drgrice1

This generally looks good except the scoring page. I am not entirely sold on centering the footer relative to the left part of the page content for the pages that have the info box, but I can live with it.

drgrice1 · 2023-12-03T23:56:51Z

templates/ContentGenerator/Instructor/Index.html.ep

@@ -19,6 +19,8 @@
 	<%= maketext('Select user(s) and/or set(s) below and click the action button of your choice.') =%>
 </p>
 %
+% stash->{footerWidthClass} = 'col-xl-10 col-md-12';


The col-md-12 is not actually needed. The col-xl-10 class does not have effect if the window width is below the xl breakpoint, and for anything under that the div will have width:100% set (any direct descendant of a row gets that). It doesn't hurt, but it isn't needed either.

drgrice1 · 2023-12-04T00:07:09Z

templates/ContentGenerator/Instructor/Scoring.html.ep

+			<%= $c->hidden_authen_fields =%>
+			<%= hidden_field returning => 1 =%>
+			<div class="row">
+				<div class="col-sm-6 mb-2">


I don't really like how this page looks the best with the change here to a col-sm-6 and the later change from col-sm-7 to col-sm-6. The styles as they were did a better job of preventing the radio button labels from wrapping. The only wrapping that occurred before was of the "Include percentage grades columns for all sets" label on the bottom, and that only happened for narrow windows and that didn't look that bad. I don't really thing the set names need that much width in most cases anyway.

In general, I don't think that the changes to this file look good at all. After scoring several sets this looks quite unbalanced. Here is a screenshot:

The hr above the scoring results goes the entire width of the page, and also doesn't look right even if the scoring region isn't so wide.

pstaabp · 2023-12-04T20:58:42Z

I agreed with @drgrice1 about the footer on the scoring tools page, but other than that, this looks good.

Alex-Jordan · 2023-12-05T00:11:14Z

The last commit here gets a bit off topic, and makes some styling changes to the Scoring template. For now it still has the footer center aligned with the control panel at the top.

I'm happy to take feedback about the styling changes and change them. And if it's the majority opinion to center the footer with the page instead of with the control panel, I'll change that too. I was trying to avoid the status quo that is like:

where the footer feels mis-positioned. But I know it might just be trading one bad thing for another.

drgrice1

I am not liking the new layout for the form on the scoring page.

I do like the lists for the set scoring file downloads though.

drgrice1 · 2023-12-05T02:06:53Z

templates/ContentGenerator/Instructor/Scoring.html.ep

+			<%= $c->hidden_authen_fields =%>
+			<%= hidden_field returning => 1 =%>
+			<div class="row">
+				<div class="mb-2 col-6">


The changes here and the wrapping row and col divs above make wrapping of the radio button labels even worse. Even with the shortened labels. I also don't like that the border is now flush up against the site nav. Generally, borders don't work very well on the div's that are part of a bootstrap grid layout. You are probably going to need to add another div inside grid layout div's that contains the form, and put the border on that. The following screenshots are taken at the same window widths, the first with this pull request, and the second with the develop branch.

Also, I really don't like that you removed the sm breakpoint from this. Below the sm breakpoint (the xs layout) is generally for mobile devices. Here is what things look like with this pull request compared to develop on an android device in Google Chrome:

Notice that selects behave differently on mobile devices, and it is better if this is not in the col layout.

drgrice1 · 2023-12-05T02:26:37Z

I added a pull request to this branch that fixes the things I mentioned in one way.

Alex-Jordan · 2023-12-05T04:42:17Z

Thanks for that, it's merged now and it looks better.

Still accepting opinions if the footer should go back to being col-12 or stay aligned with the controls.

drgrice1 · 2023-12-05T12:14:54Z

Still accepting opinions if the footer should go back to being col-12 or stay aligned with the controls.

I am okay either way on that. The issue is that no matter what you choose, the footer will not always be aligned with whatever is directly above it (with the current layout -- see below). If there are no scoring files, then the scoring section below is not shown. If there are scoring files, then the data shown may be wide or narrow depending on the contents of the scoring file, and generally aligning the footer with that is completely impossible since fit-content is used.

One option would be to remove width:fit-content; max-width:100%; style that was added to the scoring file content display, and change to $c->stash->{footerWidthClass} = -f "$scoringDir/$c->{scoringFileName}" ? 'col-12' : 'col-md-12 col-lg-10 col-xl-8'; for the footerWidthClass. Then when the score file contents are not shown, the footer will be centered below the form, and when the scoring file contents are shown the footer will be centered below the full width pre containing the contents.

Alex-Jordan · 2023-12-16T20:24:24Z

This had conflicts that I just resolved with a rebase and some editing. It was just about the presence of the word "at" in the footer for "Page generated ...".

Alex-Jordan · 2023-12-16T20:54:22Z

I never saw or used the s...scr.csv or s...ful.csv files before working on this page for this PR. Such files are not created until you check the box "Record scores for each set" and then score the selected sets. I get it that the purpose is to score lots of sets all together, but also created the individual csv files for each set. Those could be used in a mailmerge, for example.

I am not really sure why have two of these. Did only the scr variant exist at first, then ful was added but to not break people's mail templates, they kept both?

Anyway suppose you select sets A, B, C, and check the box "Record scores for each set", and click to score. Next week you come back and you select only set C, and you do not check that box, and you score. The two files sC....csv have not been updated. You are shown the main scoring csv file you just created for set C, and also shown links for the two old files for set C, and it is suggested you can use these in a mailmerge. You are misled to think those have been updated.

I'm not going to mess with any of this, but it seems like it needs attention. Possibly revisiting how many of the features here are useful and/or actually used.

Alex-Jordan · 2023-12-16T21:02:12Z

One option would be to remove width:fit-content; max-width:100%; style that was added to the scoring file content display, and change to $c->stash->{footerWidthClass} = -f "$scoringDir/$c->{scoringFileName}" ? 'col-12' : 'col-md-12 col-lg-10 col-xl-8'; for the footerWidthClass. Then when the score file contents are not shown, the footer will be centered below the form, and when the scoring file contents are shown the footer will be centered below the full width pre containing the contents.

I tried this. It rarely applies the col-md-12 col-lg-10 col-xl-8 class. It only seems to do so if I clear the scoring/ folder. So it does align the footer well the first time an instructor visits scoring tools, but in practice after that it will always be col-12.

Another thing about this I was trying to do, is when the csv file being shown is not wide, having it artificially extend all the way to the right of the page seems wrong. The illusion of that happening is there now because I gave the pre a background color. The width:fit-content; max-width:100%; constrains the pre so it won't get that wide, and the gray background is more of a tight hull surrounding the csv file. All that said, it's not important to do this. I could do what you suggested and live with the extra gray "padding" of the csv presentation. I'll take anyone's expressed preference about it (and sorry of that's already been given but I missed it.)

This sets a default `footerWidthClass` stash value to `col-12` in the ContentGenerator `go` method. Any module or template can modify that as needed to match its layout. This is done in `templates/ContentGenerator/Instructor/Index.html.ep`, `templates/ContentGenerator/Instructor/Stats/index.html.ep`, and `templates/ContentGenerator/Problem.html.ep`. Note that this can also be done directly in the perl code for any package that derives from the ContentGenerator as well (i.e., in a `.pm` file). To do so call `$c->stash->{footerWidthClass} = '...'`. I have left the remaining pages for you to determine, since I am uncertain what you will want on some of them.

drgrice1 approved these changes Dec 2, 2023

View reviewed changes

drgrice1 reviewed Dec 4, 2023

View reviewed changes

Alex-Jordan force-pushed the footer branch from 18d20fd to a42fa92 Compare December 5, 2023 00:01

drgrice1 reviewed Dec 5, 2023

View reviewed changes

Alex-Jordan force-pushed the footer branch from a68f261 to 029e05e Compare December 16, 2023 20:23

Alex-Jordan force-pushed the footer branch from 029e05e to 6adda4a Compare January 10, 2024 04:50

Alex-Jordan and others added 5 commits January 19, 2024 22:33

reposition footer

667927d

more footer position adjustments

941bb53

more changes to Scoring template

70ee9e4

A few style tweaks to the layout of the form on the scoring page.

896ab2c

Alex-Jordan force-pushed the footer branch from 6adda4a to 896ab2c Compare January 20, 2024 06:34

pstaabp approved these changes Jan 24, 2024

View reviewed changes

pstaabp merged commit b2b054f into openwebwork:develop Jan 24, 2024
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

reposition footer #2269

reposition footer #2269

Alex-Jordan commented Dec 2, 2023

drgrice1 left a comment

drgrice1 Dec 2, 2023

Alex-Jordan commented Dec 2, 2023

Alex-Jordan commented Dec 3, 2023

drgrice1 commented Dec 3, 2023

drgrice1 commented Dec 3, 2023

Alex-Jordan commented Dec 3, 2023

drgrice1 left a comment

drgrice1 Dec 3, 2023

drgrice1 Dec 4, 2023

pstaabp commented Dec 4, 2023

Alex-Jordan commented Dec 5, 2023

drgrice1 left a comment

drgrice1 Dec 5, 2023

drgrice1 commented Dec 5, 2023

Alex-Jordan commented Dec 5, 2023

drgrice1 commented Dec 5, 2023

Alex-Jordan commented Dec 16, 2023

Alex-Jordan commented Dec 16, 2023

Alex-Jordan commented Dec 16, 2023

	<div id="footer" role="contentinfo" class="row"><%= include 'ContentGenerator/Base/footer' %></div>
	<%= include 'ContentGenerator/Base/footer' =%>

reposition footer #2269

reposition footer #2269

Conversation

Alex-Jordan commented Dec 2, 2023

drgrice1 left a comment

Choose a reason for hiding this comment

drgrice1 Dec 2, 2023

Choose a reason for hiding this comment

Alex-Jordan commented Dec 2, 2023

Alex-Jordan commented Dec 3, 2023

drgrice1 commented Dec 3, 2023

drgrice1 commented Dec 3, 2023

Alex-Jordan commented Dec 3, 2023

drgrice1 left a comment

Choose a reason for hiding this comment

drgrice1 Dec 3, 2023

Choose a reason for hiding this comment

drgrice1 Dec 4, 2023

Choose a reason for hiding this comment

pstaabp commented Dec 4, 2023

Alex-Jordan commented Dec 5, 2023

drgrice1 left a comment

Choose a reason for hiding this comment

drgrice1 Dec 5, 2023

Choose a reason for hiding this comment

drgrice1 commented Dec 5, 2023

Alex-Jordan commented Dec 5, 2023

drgrice1 commented Dec 5, 2023

Alex-Jordan commented Dec 16, 2023

Alex-Jordan commented Dec 16, 2023

Alex-Jordan commented Dec 16, 2023