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

Theming reloaded #122

Merged
merged 70 commits into from Apr 1, 2019

Conversation

Projects
None yet
4 participants
@alexAubin
Copy link
Member

commented Feb 21, 2019

https://hooktube.com/watch?v=FDkhqjvS43E

Sooo I tried to merge the various work from #116, #117 , #118 and #121 because it was quite confusing because there are many different things to handle (and the specs of theming were not so well-defined) ... There are also various things which I am not sure to understand, mostly related to the historical code for SSOwat...

TODO :

  • check / include chateau theme example
  • have a look at all the FIXME's
  • test in various case (logged in / not logged in, check the 'yunohost' button and overlay, ...)
  • Try to remove some JS madness
  • Implement a nice "light" theme (in opposition to the default 'dark' theme)
  • Write some documentation on how to add / build a new theme
  • There is a weird issue when logging-out from the 'overlay' thing : you don't truly get redirected to the actual login form, you stay in the overlay ... Probably related to the fact that it's done in an iframe
  • Make it easy to change the yunohost logo by something else
  • Make it easy to change the yunohost background by something else
  • We might want to tweak yunohost_panel.conf.inc in core instead of loading 3 different piece of js/css from the JS, otherwise that's a huge mess ...
  • Maybe we should split custom.css (from themes) into a custom_portal.css and custom_ynhicon.css ... to be discussed

Test cases to test / validate :

  • Login page
  • Portal page
  • App page when it include ynhpanel.js (the icon in the corner)
  • The portal overlay when clicking on the icon in the corner when being in an app
  • All of those both for the default theme and the example themes ...
  • Annnnnd check that everything looks decent on mobile ...
Show resolved Hide resolved portal/assets/js/global.js Outdated
Show resolved Hide resolved portal/assets/js/global.js Outdated
Show resolved Hide resolved portal/assets/js/global.js Outdated
@lfuelling

This comment has been minimized.

Copy link
Contributor

commented Feb 22, 2019

Hey @alexAubin,
I added some comments to the sections I wrote. Hopefully it clarifies their purpose.

Show resolved Hide resolved portal/assets/js/global.js Outdated
Show resolved Hide resolved .gitignore
Show resolved Hide resolved portal/assets/js/global.js Outdated
Show resolved Hide resolved helpers.lua Outdated
@@ -11,5 +11,8 @@

<!-- Scripts -->
<script src="assets/js/global.js"></script>
{{#theme}}
<script src="assets/themes/{{theme}}/global.js"></script>

This comment has been minimized.

Copy link
@lfuelling

lfuelling Feb 22, 2019

Contributor

So every theme has to implement Javascript or the request for this element will fail. (Which is only visible when the devtools are open but imo it's a bad practice.)

Would it be possible to detect if a js file exists for given theme and only put in the script tag if it does?

}
// if asked in current tab
else {
event.preventDefault();

This comment has been minimized.

Copy link
@lfuelling

lfuelling Feb 22, 2019

Contributor

I would add a call to event.stopPropagation to prevent the event from bubbling up.

@alexAubin

This comment has been minimized.

Copy link
Member Author

commented Mar 19, 2019

Sooooo let me try to summarize the various things I've been working on :

  • split the CSS into two specific files : portal and overlay - so that you don't have to make sure that you rules for the portal won't cause any trouble on apps ...
  • however I kept a single JS files ... partly because of laziness and also to avoid an additional request / file to handle ... also because the portal and overlay kinda use the same helpers so splitting the file would mean duplicating code
  • implemented a light theme (in opposition to the default "dark" theme) : c.f. screenshot of login and app list - inspired by what I saw a while ago on Open Tunisia
  • removed some JS madness loading the additional CSS and JS after DOMready event ... c.f. YunoHost/yunohost#689
  • fixed an issue with dragging of the small yunohost button/tile inside the overlay iframe
  • enabled a 1-hour cache on all the portal assets to reduce the number of request ...
    • I choose 1 hour because some of those assets may change (e.g. if the admin changes the theme or just the logo .. you don't want to have to tell all your user to force a cache refresh ... I guess 1 hour is an acceptable tradeoff)
  • many attempts to improve the semantic of various stuff, + various small tweaks / bugfixes
  • documented the whole thing, c.f. : YunoHost/doc#957

Soooo to me the work is complete. The last thing I'm wondering about is what themes to really include. Imho the light theme is definitely to be kept (apparently we tend to underestimate how dark interface make people afraid...). The clouds and vapor theme are kinda fun and show example of what can be done. However imho the random theme imho is a bit too much... so I propose to remove it (or it should be optimized to avoid copypasting n time the logo just to change the color :s ).

@eauchat

This comment has been minimized.

Copy link

commented Mar 19, 2019

Hey, some quick remarks...
It's really cool all the work :)
I totally agree with you that it's not necessary to include the random theme by default. It needs improvement, and I think it's place is more in a repo somewhere with custom themes that people can install if they want.
One proposal: let's rename the info.html page to portal.html, would be more clear no?
And it's been a while that I was thinking it would be also cool somewhere for admins to actually be able to set a completely custom portal page. In other words, instead of /yunohost/sso/ serving info.html, it would serve whichever page the admin decided/created instead.
I think it would be really cool, and I imagine it might not be so complicated to do. But maybe it's not actually something to do in sso repo but in yunohost repo?

alexAubin and others added some commits Mar 19, 2019

Merge pull request #132 from eauchat/theming-reloaded-redirections
Theming reloaded redirections improved
@alexAubin

This comment has been minimized.

Copy link
Member Author

commented Mar 25, 2019

Planning to add this to the 3.5.x which I would like to release early next week ... so this should be merged in the next few days

@alexAubin alexAubin merged commit a194168 into stretch-unstable Apr 1, 2019

@alexAubin alexAubin deleted the theming-reloaded branch Apr 1, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.