search

thexerteproject / xerteonlinetoolkits

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

Learning Object Title link to home page #976

Closed JohnSmith-LT closed 2 years ago

JohnSmith-LT commented 3 years ago

This was added as an enhancement request to the Forum; question is are there any good reasons not to or to allow as an option?

https://www.xerte.org.uk/index.php/en/forum/wish-list/2282-learning-object-title-link-to-home-page#7013

John

JohnSmith-LT commented 2 years ago

Clearing stuff up @FayCross @ronm123

Can you think of any good reason not to add this as an optional property?

The solution is already there on the Forum...

FayCross commented 2 years ago

I don't think it would cause any problems to do this - unless it makes things confusing for accessibility (can we make it obvious what this link would do for screen readers?)

There's an optional property already called home page which allows you to change the page (when navigation is historic) that the home button links to (by default it's the 1st page). The title link should probably be the same - use 1st page unless home page is changed in that drop down. Both properties should probably be in an optional property group together.

JohnSmith-LT commented 2 years ago

Sounds good @FayCross, just going through older issues to see if any of them are no longer relevant and trying to get a set that I can work on to try to get the number down again.

I'll add this and come back with something that you can cast an eye over.

JohnSmith-LT commented 2 years ago

Have added some code @FayCross - not complete but can you cast a quick eye over it to see if this is what you intended. @ronm123 do you have any suggestions?

I know it most likely still needs work for the other themes etc... but what about functionality and/or editor layout, etc

ronm123 commented 2 years ago

@JohnSmith-LT @FayCross Just tested this and seems to work well. A few comments...

At the moment the tooltip on the link says Goto Home page - I wonder if that should be Go to Home page e.g. with a space between Go and to?

I wonder if we should add a tooltip to the wizard for the Link LO title to Home Page checkbox to make it clear that if ticked the LO title will link to the home page or first page whatever the navigation option is set to? e.g. that current home page option where you can change the destination only applies if navigation is set to Historic but the LO Title link works regardless of the navigation setting. At the moment checking the box makes the LO title a link to whatever homepage is set if navigation is historic but the first page if navigation is anything but historic. I understand why the change of destination has only ever worked if navigation is set to historic but I think now that we have the additional checkbox so that the title becomes a link, then that link should go to the first page by default, but to whatever is set as the home page if that option has been changed and regardless of the navigation setting?

So if you agree with the above then: The tooltip for the homepage option should change to something like: Homepage used by the home button when navigation is set to 'Historic'. By default this is the first page in the project.

The tooltip for the checkbox should be something like: Check this box and your LO title will be a link to the first page (by default) or whatever is set as the homepage above. This works for all navigation options.

@JohnSmith-LT I haven't checked the LO Title link for all themes but unless you are setting a link colour in the code for this I think most themes should have an appropriate link colour for the header anyway although I could be wrong about that so we should check with all themes I guess.

ronm123 commented 2 years ago

BTW once complete we should add this to the release notes and I can undertake to do that and I'll also check with all the bundled themes for any header link colour issues. @FayCross I know you've got custom themes at Nottingham so you'll need to check those?

FayCross commented 2 years ago

@ronm123 Yes, I agree with your comments above and was already making these changes as you sent this. I'll commit in a minute so could you test it works as you'd expect after that please. I've changed so that home page drop down is a conditional property which only shows when it will be used. So it appears when navigation set to historic or if link LO title checkbox is ticked. If navigation is set to menu/menu with page controls then it won't show and home link will always link to page 1 (the table of contents). I think linking to other pages would cause confusion in these cases.

I haven't looked at any themes yet.

FayCross commented 2 years ago

@ronm123 feel free to change tooltip text - I wrote it before I read your comments above

ronm123 commented 2 years ago

@FayCross @JohnSmith-LT I've checked all the bundled themes and a few custom one that I have. There only seems to be an issue when the header colour isn't dark and therefore the header text isn't white e.g. futureteacher white on white but focus ok orangeandpurple beige on beige but focus ok flat white white on white but focus ok informal sketch white on white but focus ok django blue on blue but focus ok

ronm123 commented 2 years ago

@FayCross I've pulled down your changes and the tooltips seem fine but I'm not sure about the conditional property change....

Do you have existing projects set to historic where the homepage option has been changed and do they still work as they did?

Also isn't there a scenario where for those existing projects set to historic navigation and/or new projects set to historic navigation that the author wants the homepage button to go to selected homepage but doesn't want a homepage link attached to the LO title? Without the condition added that was possible but now it isn't?

FayCross commented 2 years ago

@ronm123 That still works doesn't it? If I change the navigation to historic, then add home page optional property then I can see the drop down menu even if the check box isn't checked (& when I preview the home btn goes to page I've specified and the title isn't a link). If I change the navigation to linear then when I add the home page property I don't see the drop down until I check the box. Is it not working like that for you?

ronm123 commented 2 years ago

@FayCross ah sorry I hadn't realised that you changed it so that the navigation setting also changes what is shown. I had my test changed to linear and so with the checkbox unchecked wasn't seeing the homepage menu. :-(

Yes it works great then and I guess you've checked existing projects are ok.

With the theme issues I have listed with a small number of themes is the only way to deal with that to change it in each of those themes or is there a better way?

FayCross commented 2 years ago

@ronm123 - yes, it works for historic projects that are already using home page functionality.

I changed themeStyles.css earlier so it shouldn't require changes to individual themes - I've only check UoN ones so far though so worth having a look at the ones you found problems with earlier again

JohnSmith-LT commented 2 years ago

Thanks @FayCross and @ronm123 I wasn't quite sure how to finish this one off or make it work best with the themes but that all seems to work and look great now.

ronm123 commented 2 years ago

Thanks @FayCross @JohnSmith-LT I just checked the themes I listed and the only one that still has an issue is Django because it has the color set in the following: a:-webkit-any-link { color: #007cdb !important; text-decoration: none !important; } in /django/scss/_webkitCustoms.scss

That's a theme contributed by one of Tom & Inge's colleagues but I guess an easy fix would be to exclude #x_headerBlock from that rule or add an additional rule after that one to change #x_headerBlock a back to white?