thexerteproject / xerteonlinetoolkits

Xerte Online Toolkits
www.xerte.org.uk
Apache License 2.0
62 stars 61 forks source link

Responsive text size - work in progress... #387

Closed ronm123 closed 8 years ago

ronm123 commented 9 years ago

This is less a bug and more of a question about enhancement but adding it here for our discussion tomorrow. It's always bugged me since we've moved to HTML5 rather than Flash that the poor maligned Flash Player did a great job of resizing the stage and text content according to browser window size when set to full screen or fill window. Obviously we have the full screen and fill window options with the HTML 5 engine too but I've noticed that most users stick with the default and I think this is because of how poor the text can look when viewed at larger window sizes. Making the default text size larger is only really a compromise especially as making it large enough for a high res desktop is then way too large for smartphones etc. I've been working through most of the page types and checking where there are unique font size styles that need to be included in a more responsive stylesheet which has taken quite a while and I may have missed some but here's a comparison for discussion tomorrow.

View the Nottingham page types example http://www.nottingham.ac.uk/toolkits/play_8203 and compare each page with this example http://training.mitchellmedia.co.uk/xot/play.php?template_id=121 on various window sizes and devices. At the moment I've only set the styles to kick in at 768 width and above so lower width than that defaults to the existing styles especially on phones etc. But for me this seems to look and work much better on iPad and desktops when the LO is set to fill window. So far it's just the text that is changed not any of the current image sizes etc. I'll paste a couple of screenshots from my wide screen laptop... image image

BTW - wish we could copy and paste images into the Xerte editor like we can here on github!

thexerteproject commented 9 years ago

I think this is a definite improvement, I think that was the consensus when we spoke on Friday as well. Perhaps we need a 'responsive' branch for this piece of work?

ingedonke commented 8 years ago

This looks good! What is the status on this? I have a question from someone about the text size in the modal window. Do you have a quickfix for this? Or a stylesheet I can use for now?

ronm123 commented 8 years ago

I've been working a bit more on improving this as part of a project I'm supporting. The responsive text part of this is working well now and I've been working with a derivative of the Flat White (no borders) theme so I think the first step might be to add that to the themes and get you to test it on your own projects. I've been using a snippet of css within different pages to make the images on a given page responsive too. This also seems to work well but longer term we need a better way to make that FWS. I'll tweak this example and commit a derivative theme as soon as I can but will probably be the end of the week.

@ingedonke can you explain more perhaps with a screenshot what modal window and text size you mean?

ronm123 commented 8 years ago

Hi again quick update. I've added the css changes I've made so far to the end of the Flat White theme and here's a link to an example with that applied http://training.mitchellmedia.co.uk/xot/play.php?template_id=127 shall I commit that change to the flat white theme to develop?

ronm123 commented 8 years ago

Hi all as discussed briefly with Julian and Fay just before the Apereo webinar on Wednesday I've been quite happy with the way the responsive text additions to the themes has been working apart from the issue when the LO first loads that the page contents displays too high and overlaps the title until moving to the next page or resizing the window slightly. Fay suggested this might be to do with an icon in the header but I've tested and this happens regardless of whether an icon is added and also regardless of what the page title text size is. I've looked through the css and can't find anything likely to be causing this directly but might be missing something? I wondered if there is anything that loads/triggers when the default theme is used compared with the other themes or directly added css? Can someone else have a look as I might not be seeing the wood for the trees now! :-( I've attached a css file saved as txt because github doesn't allow css attachments. If you rename this and add it to an LO with the default theme as an optional property and then view the LO the same thing happens but I'm not sure if that because of a style or just because of the order of loading etc.

set_font_sizes_to_vmin.txt

ronm123 commented 8 years ago

Just to make it clear here's a screenshot of the problem with the first page of the LO being a tabbed navigator image

FayCross commented 8 years ago

Hi Ron, I'm looking into this now. There seem to be a couple of related issues here - the icon does cause similar problems in even the default theme if it's taller than the header bar would be without it but other themes have the problem regardless of whether they contain the icon.

This is unrelated to this bug but a question about the responsiveness code. Do you think it should only affect projects that are set to fill the screen? I don't think it works as well for projects displaying as the default 800 x 600 size as if you're on a large screen the text appears much too big:

xot

ronm123 commented 8 years ago

Hi Fay thanks for looking at this - have you found a solution? Regarding the question about the default screen size setting resulting in the text sizes being too big yes I wasn't sure how to deal with that. If a project is set to full screen (or fill window) and then the browser window resized down to 800 x 600 the text size is responsive and not too big but when set to default and viewed in a much larger window the css obviously picks up the window width rather than the Xerte div width. Is there a way to deal with that whilst still keeping the css in individual themes?

FayCross commented 8 years ago

Hmm, I can't think of a way around that. Ideally we'd have a separate responsive css file with the other main css files (mainStyles.css, desktopStyles.css etc.) and then we could add or remove the link to responsive.css depending on whether we wanted the project to be responsive (perhaps controlled by a checkbox in editor, whether project is currently default or fill screen view, or even a flag in the theme.info files).

I can't think of a way to have it within a theme and have that kind of control over it.

ronm123 commented 8 years ago

Hi Fay it would be easy to separate out the responsive text css from each of the themes because at the moment it's simply added at the end of the original styles. So how about this...

  1. We add responsivetext.css to each of the themes (including the default theme) and remove those styles from the end of the theme css e.g flatblue.css
  2. Add learning object property to disable or re-enable the responsive css.
  3. Add the code to detect the chosen setting but override if the project display size is set to default. Also re-enable if the user selects the maximise button if possible?

I think with 2. the responsive text css should be enabled by default and the tick box should be set and displayed rather than hidden away in optional properties?

I did wonder about having a single responsivetext.css somewhere rather than one in each theme but ultimately I think they may need to vary a bit and be controllable by theme. However would be good to crack the issue with overlapping the page titles first if that's a css rather than js issue?

FayCross commented 8 years ago

It was a js problem & I've committed a fix for it. The height of the central section where the pages load has its height & margins set in js as it wasn't possible to get the layout we needed just with css. It looks at the height of the header & footer bars to do this but the extra css (and icons) that could change these heights weren't always loaded when it was calculating. It now waits for all this to load first. I've tested it in FF, Chrome, IE11, iPad Safari and it seems ok now (for full screen anyway).

Your 3 steps make sense. Ideally it would be best to have one central file but I understand why you say it might not work. is it over complicating things to have one responsivetext.css file and then only have a responsivetext.css file in a theme if it does need to be different in someway? I take it that the responsive code in each theme is the same at the moment?

ronm123 commented 8 years ago

Hi Fay that's great that you've found the issue and fixed it. I'll download and test later. Yes you're right it would be better to include just one responsivetext.css unless a theme needs changes but I guess we need to be careful what order that is loaded. e.g. if the 'master' responsivetext.css was in the default theme it would still need to be loaded after for instance flatblue.css otherwise the sizes could be overridden? Also are there implications for import?

At the moment I think all the themes have the same added css but I seem to recall there was one that was different which might have been the sketch theme but I'll go through and check. Might just be easier to include in each theme? I'm happy to go through and strip out the css into separate responsivetext.css files in each theme but I guess we need to coordinate doing the separate bits around the same time?

FayCross commented 8 years ago

Fair enough, you're probably right that it would get too complicated to manage what should when. I'll change the xenith code today to also load responsivetext.css files from themes as there's no real harm in doing this in develop before those css files exist. I'll leave adding a property in the xwd until you've created the css files though (or you can do this when you're ready).

The thing to think about then is how to handle existing projects. We can force the responsive check box into the editor by making it mandatory but if you want it checked by default then it won't actually work for existing projects unless it's unchecked and then checked again (it's only at this point that it would be inserted into xml). I don't suppose we would want to force all old projects to be responsive without having it in the xml though. Anyway - you can think about how to handle this later!

ronm123 commented 8 years ago

Hi Fay I've just committed the separated responsivetext.css files. Yes agreed we probably don't want to force the responsive text for existing projects but it would be good to have it the default for new projects. I guess anyone who has downloaded and installed develop since the responsive styles were in place won't have too many projects that would need updating by selecting the checkbox. So would this work e.g. unchecked for existing projects but checked for new projects?

FayCross commented 8 years ago

Ignore me - I was thinking it would have to be checked for both which would be confusing as it wouldn't work for existing unless you unchecked and checked again. But I think I was wrong actually and we can have checked for new and unchecked for existing. I'll let you know when I've made the changes to it all and you can test it.

FayCross commented 8 years ago

I've just committed the changes that make this work. I've tested in a few browsers and with all your themes and the only problem is with the Black & white theme but this is because there's still some responsive code in the main css file.

Let me know if you find any problems with it when you test it. It looks at a responsive property in xwd and whether display mode is default or fill/full.

FayCross commented 8 years ago

Also, I haven't added anything in yet that will deal with responsiveness in the default template as I don't think a responsivetext.css file exists for this yet. So if you leave it on defaut theme & tick responsive it won't do anything. i can add this when the css file is ready.

ronm123 commented 8 years ago

Hi Fay just tested and works great! I removed the left over css from the blackround.css and added a copy of responsivetext.css to \modules\xerte\parent_templates\Nottingham\common_html5\css\ Great that this can be a simple on/off option for all those existing projects using the default theme and that it kicks in if set to responsive when a user maximises the display even if set to default by the author.

FayCross commented 8 years ago

I haven't looked into this but if you insert a plain text page and preview without changing anything in the editor then the main text doesn't become responsive. Is this because the css look for p tags? If it does then it might not work particularly well in old projects made in the old editor where p tags won't automatically be present. You might want to test this a bit to see if this is a problem.

Glad it works for you! I'll add it to the default theme too but probably tomorrow now

ronm123 commented 8 years ago

Fay - whilst we're working on this a few related items...

  1. Should we add an empty themename.js file to each theme perhaps containing a comment //add custom js/jquery here? I know it's a minor thing but it currently throws a 404 in console etc and I guess in server logs too. I think this is because the code isn't checking if the js file exists before loading it?
  2. I think we've discussed somewhere before but can't find it the idea of adding a css/script option to the Nottingham template like we have in the bootstrap template e.g. an optional textarea where custom css can be added directly which applies across the whole project rather than just the page to which it is added.
  3. Related to 2. in a way is that if styles or code is added to the learning object title field this shows as code in the title of the LO in the browser and when saving as a favourite etc. Should we add something to xenith.js to extract just the title and ignore any code?
ronm123 commented 8 years ago

Hi Fay yes it does only work where there are tags and does mean that there can be a mix of sizes in existing projects/pages that haven't been edited. Not sure there's a way around that without unpredictable results? However I did test with a plain text page and pasted some text and that worked because it automatically added

tags. How did you add a new plain text page and added text without getting

tags?

FayCross commented 8 years ago

No sorry, I noticed it because I hadn't changed the text - it still has default 'Enter text...'. It just made me think that it might not work that well in old projects after all.

FayCross commented 8 years ago

In answer to your other points: 1 & 2. Yes - I'll do these

  1. Do you mean when that text appears in the html title tag? I'll have a look at it
FayCross commented 8 years ago

Ron - quick question. Do you want the common_html responsivetext.css file to load even when the default theme isn't being used? If only when default theme is selected then what if the theme doesn't contain a responsivetext.css file?

ronm123 commented 8 years ago

Hi Fay If I understand your question correctly I think the one in common_html should only be used if that theme is used. Any themes we release should have responsivetext.css even if different or commented out. Any themes others develop and add to their own installs will be up to them?

FayCross commented 8 years ago

Yes that's what I was asking. I just wanted to check as common_html5/css/themeStyles.css is always used even if a different theme is selected too. I'll add this code now

FayCross commented 8 years ago

I've made all the changes now @ronm123 including the 3 extra bits you asked for above. Let me know if you have any problems with any of it.

ronm123 commented 8 years ago

Thanks Fay will have a look and respond accordingly...

ronm123 commented 8 years ago

Hi Fay the new styles panel works great. At some stage we should probably change the bootstrap styles panel to that format because at the moment it shows the normal text area which means having to switch to source view. The stripping of code from the LO title works but only if there is code in the title. If there isn't it seems to flash up and then disappear? e.g. with a new lo and default title it flashes up Learning Object Title then reverts to the url.

FayCross commented 8 years ago

I've fixed that now

ronm123 commented 8 years ago

Hi Fay/Tom I'm not sure if this is due to the code Fay added to strip code from the LO title or whether it's due to the change of the trim function, or it may be neither, but I just noticed that if there is a forward slash in the LOT title (e.g. my test/demo) it stops the LO from displaying with the following errors: Line: 4 Error: Syntax error, unrecognized expression: my test/demo SCRIPT5022: Syntax error, unrecognized expression: my test/demo jquery-1.9.1.min.js, line 4 character 10920 {throw Error("Syntax error, unrecognized expression: "+e)} I've tested the same in an earlier version 3 install and no such problem. Can you replicate this in develop? If this is due to the change to remove code from the lo title is there a solution that retains that functionality? Probably better to not strip the code if it risks breaking LO's with forward slash in the title etc? Not sure if any other chars cause a similar problem but tested backslash and that doesn't cause a problem.

torinfo commented 8 years ago

Fixed this. This could potentially be a problem in toolbox.js as well.Somehow it works there because of the

tag that the editor puts around the text

ronm123 commented 8 years ago

Have just tested Tom and can confirm that the forward slash in an LO title doesn't break playback any longer.

ronm123 commented 8 years ago

I think this is all in place now so closing the issue.