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

new user interface #717

Merged
merged 192 commits into from Aug 1, 2018

Conversation

Projects
None yet
9 participants
@Olimpiamagliulo
Collaborator

Olimpiamagliulo commented Nov 9, 2017

Change the design of SSP starting from index.php, authentication method and login page.

  • Based on appearance-config the user can display the standard theme or the new one
  • With the new theme the user is redirected to login page or can choose an authentication method
  • Migrating php templates to twig
  • Languagebar becomes a dropdown menu.
  • Restructure css and js files from www/resource/ into www/assets/
  • New user interface will focus on intuitive design and responsiveness.
  • Future changes will follow mvc-pattern.
  • Use yarn for package management-> .gitignore updated accordingly.

aguvillalba and others added some commits Oct 16, 2017

Implement "Remember my Organization" in multiple-ldap
This new feature allows the users to have a checkbox in the login form
which works in the same way than "Remember my username" but for the
organization selected in the dropdown
Implement "Remember my Organization" in multiple-ldap
This new feature allows the users to have a checkbox in the login form
which works in the same way than "Remember my username" but for the
organization selected in the dropdown
Implement "Remember my Organization" in multiple-ldap
This new feature allows the users to have a checkbox in the login form
which works in the same way than "Remember my username" but for the
organization selected in the dropdown
Olimpia Magliulo Olimpia Magliulo
Change .gitignore
ignore files generated with yarn (node.js).
Olimpia Magliulo Olimpia Magliulo
Add index.php
Based on appearence configuration the user is redirected to login.php when usenewui is set to true,
or to frontpage_welcome.php for the default design.
Olimpia Magliulo Olimpia Magliulo
Add login.php and related twig template
Based on authenticate.php, login.php adds lines 6-26.
The script returns the possible authentication methods for the user, excluded admin.
When only one method is available the user is redirected to the one authentication-method login page.
Calls to right templates are updated at lines 17 and 54.

When no method is found, the user needs to login with admin credentials and check configuration.
The text shown in template is not translated.
Olimpia Magliulo Olimpia Magliulo
Restructure css and js
Change the links in the head and postload of base.twig according to the new structure and the recently added packages.
Olimpia Magliulo Olimpia Magliulo
Add _table.twig
the template can be used in different templates.
It highlights rows differently if they're odd or even.
It has 2 flexible blocks that can be used to override a column or add other content.

Classes can be used to readapt it for a specific css framework.
Olimpia Magliulo Olimpia Magliulo
Add auth_status.twig
Build from status.php uses twig to show the authentication status.
Olimpia Magliulo Olimpia Magliulo
Languagebar to dropdownmenu
Languagebar is changed to a dropdownmenu.
Use selectize package to show the current language as the first option in the menu.
Olimpia Magliulo Olimpia Magliulo
Use queryparams to change language after a login error
Query parameters are passed to the template in order to change language also after an error is shown.
Olimpia Magliulo Olimpia Magliulo
Fix a problem when trying to change language after a login error
Fix: Missing AuthState parameter when trying to retrieve the authentication state.
Olimpia Magliulo Olimpia Magliulo
Use grids to style the other elements in the base template
Each element is defined with an id. it has its own wrap and a "row".
Olimpia Magliulo Olimpia Magliulo
Add loginuserpass template
Changes default css to adapt the login form.
Need fix: when the user can choose an organization, the layout breaks when opening the dropdown menu.
Olimpia Magliulo Olimpia Magliulo
Change call to selectize
add document.ready function to script.js where all the calls to selectize are made.
update base.twig. Now script.js is the last script in the list.
Olimpia Magliulo Olimpia Magliulo
Stiky footer
Fixed footer at the bottom in fixed position. (+1 squashed commit)
Squashed commits:
[27ac007] footer
Olimpia Magliulo Olimpia Magliulo
Show login error in loginuserpassorg
The error is shown also if just one of the fields (user or password) is empty.
Olimpia Magliulo Olimpia Magliulo
Bug fix: change language after login error
As for loginuserpass, fixes a bug that didn't allow to change language after a login error was shown.
Olimpia Magliulo Olimpia Magliulo
Change path for default-rtl.css
Adds rtl features for form elements used in loginuserpass.

@tvdijen tvdijen referenced this pull request Jul 28, 2018

Merged

assorted fixes #899

tvdijen added some commits Jul 28, 2018

@jaimeperez

This comment has been minimized.

Member

jaimeperez commented Jul 30, 2018

Hi Tim!

Any thoughts on this @jaimeperez before I start working on it?

I agree, it's a good idea to try to clean things up to make it easier to merge this. It might also be an idea to try to work on specific things in sub-branches, then merge them to this one. Another option is to use this branch only as a reference to open new, smaller, targeted PRs that we can merge one-by-one.

@tvdijen

This comment has been minimized.

Member

tvdijen commented Jul 30, 2018

Cleaned up a fair amount already, but the next step feels like a giant leap to me...
I've tried to cherry-pick one of the Twig-templates to a copy of master and it horribly breaks on translations... Aren't Twig and new translations supposed to work flawlessly on master?

https://github.com/simplesamlphp/simplesamlphp/blob/Xnew-ui/config-templates/config.php#L730
Right now it will use Twig when a template is available, regardless of this setting.. Is this a documentation flaw, or a bug?

@lonoak

This comment has been minimized.

Collaborator

lonoak commented Jul 30, 2018

Hi Tim,

I'd say translations are broken in Xnew-ui too, at least this is what I see checking out this branch:

https://screenshots.firefox.com/6ZNsGEnoHCtSKL6P/sso.tuorganizacion.es

Is this what you are experiencing ?

By the way... how do you activate the new ui in master? I mean, there are missing pieces such as packages.json, which I think are necessary.

@jaimeperez

This comment has been minimized.

Member

jaimeperez commented Jul 31, 2018

Hi,

It's hard to say because it's been some time since the last time I was able to test the Xnew-ui branch, being dragged into many other projects, but it surprises me a bit that translations are broken since they work just fine for Feide using the master branch. Do you guys have the language.i18n.backend configuration option set to gettext/gettext, and the gettext/gettext and jaimeperez/twig-configurable-i18n dependencies installed?

@tvdijen

This comment has been minimized.

Member

tvdijen commented Jul 31, 2018

Yes @lonoak, I'm having similar issues, also with master + gettext/gettext.
This is what happens when I take frontpage_config.twig and _frontpage_menu.twig and use them on current master:

Jul 31 09:34:05 webapp-1 simplesamlphp-idp[3220]: 3 [b3c2ac9934] SimpleSAML\Error\Exception: Error 2 - gettext() expects parameter 1 to be string, array given at /apps/programs/simplesamlphp-idp/simplesamlphp/vendor/twig/twig/lib/Twig/Environment.php(467) : eval()'d code:112
Jul 31 09:34:05 webapp-1 simplesamlphp-idp[3220]: 3 [b3c2ac9934] Backtrace:
Jul 31 09:34:05 webapp-1 simplesamlphp-idp[3220]: 3 [b3c2ac9934] 16 /apps/programs/simplesamlphp-idp/simplesamlphp/www/_include.php:57 (SimpleSAML_error_handler)
Jul 31 09:34:05 webapp-1 simplesamlphp-idp[3220]: 3 [b3c2ac9934] 15 [builtin] (gettext)
Jul 31 09:34:05 webapp-1 simplesamlphp-idp[3220]: 3 [b3c2ac9934] 14 /apps/programs/simplesamlphp-idp/simplesamlphp/vendor/twig/twig/lib/Twig/Environment.php(467) : eval()'d code:112 (__TwigTemplate_40e687e11f68309a99439ca899432dde1efc7d509d74acc8067be9b421faba38::block_content)
Jul 31 09:34:05 webapp-1 simplesamlphp-idp[3220]: 3 [b3c2ac9934] 13 /apps/programs/simplesamlphp-idp/simplesamlphp/vendor/twig/twig/lib/Twig/Template.php:215 (Twig_Template::displayBlock)
Jul 31 09:34:05 webapp-1 simplesamlphp-idp[3220]: 3 [b3c2ac9934] 12 /apps/programs/simplesamlphp-idp/simplesamlphp/vendor/twig/twig/lib/Twig/Environment.php(467) : eval()'d code:169 (__TwigTemplate_565abc487e8d3f32b6effd1c4ea88134d248f10d35027dd109cc28b3f2000337::block_contentwrapper)
Jul 31 09:34:05 webapp-1 simplesamlphp-idp[3220]: 3 [b3c2ac9934] 11 /apps/programs/simplesamlphp-idp/simplesamlphp/vendor/twig/twig/lib/Twig/Template.php:215 (Twig_Template::displayBlock)
Jul 31 09:34:05 webapp-1 simplesamlphp-idp[3220]: 3 [b3c2ac9934] 10 /apps/programs/simplesamlphp-idp/simplesamlphp/vendor/twig/twig/lib/Twig/Environment.php(467) : eval()'d code:76 (__TwigTemplate_565abc487e8d3f32b6effd1c4ea88134d248f10d35027dd109cc28b3f2000337::doDisplay)
Jul 31 09:34:05 webapp-1 simplesamlphp-idp[3220]: 3 [b3c2ac9934] 9 /apps/programs/simplesamlphp-idp/simplesamlphp/vendor/twig/twig/lib/Twig/Template.php:432 (Twig_Template::displayWithErrorHandling)
Jul 31 09:34:05 webapp-1 simplesamlphp-idp[3220]: 3 [b3c2ac9934] 8 /apps/programs/simplesamlphp-idp/simplesamlphp/vendor/twig/twig/lib/Twig/Template.php:403 (Twig_Template::display)
Jul 31 09:34:05 webapp-1 simplesamlphp-idp[3220]: 3 [b3c2ac9934] 7 /apps/programs/simplesamlphp-idp/simplesamlphp/vendor/twig/twig/lib/Twig/Environment.php(467) : eval()'d code:29 (__TwigTemplate_40e687e11f68309a99439ca899432dde1efc7d509d74acc8067be9b421faba38::doDisplay)
Jul 31 09:34:05 webapp-1 simplesamlphp-idp[3220]: 3 [b3c2ac9934] 6 /apps/programs/simplesamlphp-idp/simplesamlphp/vendor/twig/twig/lib/Twig/Template.php:432 (Twig_Template::displayWithErrorHandling)
Jul 31 09:34:05 webapp-1 simplesamlphp-idp[3220]: 3 [b3c2ac9934] 5 /apps/programs/simplesamlphp-idp/simplesamlphp/vendor/twig/twig/lib/Twig/Template.php:403 (Twig_Template::display)
Jul 31 09:34:05 webapp-1 simplesamlphp-idp[3220]: 3 [b3c2ac9934] 4 /apps/programs/simplesamlphp-idp/simplesamlphp/vendor/twig/twig/lib/Twig/Template.php:411 (Twig_Template::render)
Jul 31 09:34:05 webapp-1 simplesamlphp-idp[3220]: 3 [b3c2ac9934] 3 /apps/programs/simplesamlphp-idp/simplesamlphp/vendor/twig/twig/lib/Twig/Environment.php:363 (Twig_Environment::render)
Jul 31 09:34:05 webapp-1 simplesamlphp-idp[3220]: 3 [b3c2ac9934] 2 /apps/programs/simplesamlphp-idp/simplesamlphp/lib/SimpleSAML/XHTML/Template.php:415 (SimpleSAML\XHTML\Template::show)
Jul 31 09:34:05 webapp-1 simplesamlphp-idp[3220]: 3 [b3c2ac9934] 1 /apps/programs/simplesamlphp-idp/simplesamlphp/modules/core/www/frontpage_config.php:186 (require)
Jul 31 09:34:05 webapp-1 simplesamlphp-idp[3220]: 3 [b3c2ac9934] 0 /apps/programs/simplesamlphp-idp/simplesamlphp/www/module.php:136 (N/A)
@lonoak

This comment has been minimized.

Collaborator

lonoak commented Jul 31, 2018

yes @jaimeperez, it is included in composer.json:

"require": {
    "php": ">=5.4",
    "ext-SPL": "*",
    "ext-zlib": "*",
    "ext-pcre": "*",
    "ext-openssl": "*",
    "ext-dom": "*",
    "ext-date": "*",
    "ext-hash": "*",
    "ext-json": "*",
    "ext-mbstring": "*",
    "simplesamlphp/saml2": "~3.2",
    "robrichards/xmlseclibs": "dev-master#93f8c07976b36b6050ab341ab5850484dc897ce0 as 3.0.2",
    "whitehat101/apr1-md5": "~1.0",
    "twig/twig": "~1.0",
    "gettext/gettext": "^3.5",
    "jaimeperez/twig-configurable-i18n": "^1.2"
  }

are all the versions correct?

@jaimeperez

This comment has been minimized.

Member

jaimeperez commented Jul 31, 2018

That's weird Tim. What happens if you fix the version of gettext/gettext to exactly 3.5?

The only thing I can imagine is that something is broken in later versions, but it's weird that it works just fine in Feide, where we have been using twig templates and gettext translations for a while now. I'm more inclined to think that there might be some kind of error in a template...

@tvdijen

This comment has been minimized.

Member

tvdijen commented Jul 31, 2018

Fixing the version doesn't help... I've also fiddled a bit with a fixed version of Twig, but it all makes no difference..
If this is a broken template, it has to be something in one of the base-templates..

@lonoak

This comment has been minimized.

Collaborator

lonoak commented Jul 31, 2018

@jaimeperez

What happens if you fix the version of gettext/gettext to exactly 3.5?

In brach Xnew-ui, exactly the same...

@tvdijen

If this is a broken template, it has to be something in one of the base-templates..

I support this idea... all the screens seem the same, so it must be something common...

@tvdijen

This comment has been minimized.

Member

tvdijen commented Jul 31, 2018

OK, I think I'm on to something..
I can see it working fine on June 1st (30b5663) and being broken on July 12th (f917d44)... Will try to investigate further

Update: identitfied 086881f as the culprit..

@tvdijen

This comment has been minimized.

Member

tvdijen commented Jul 31, 2018

OK something's playing tricks with me now.. When I go from bb5ad9c to 086881f, things break, but when I manually go over the changes, F5'ing on every changed file, things remain working..

@tvdijen

This comment has been minimized.

Member

tvdijen commented Aug 1, 2018

There! I nailed it! All it took was a few cups of strong coffee :D
See 1d7d0cb

@jaimeperez Apparently your custom classes have the same name as original Twig-classes.. That may not be a good idea!

tvdijen added some commits Aug 1, 2018

@jaimeperez

This comment has been minimized.

Member

jaimeperez commented Aug 1, 2018

Great! Thanks for hunting this down Tim!

@jaimeperez Apparently your custom classes have the same name as original Twig-classes.. That may not be a good idea!

That's precisely the intention. Having the exact same name makes it possible to use Twig with an alternative gettext implementation without the need to rewrite your code. Ideally, this should be fixed upstream. However, the Twig maintainers don't seem to care much about this... 😞

@tvdijen

This comment has been minimized.

Member

tvdijen commented Aug 1, 2018

No problem! If I break it, I will fix it... Eventually... ;)
We're again one step closer to merging this

tvdijen added some commits Aug 1, 2018

@tvdijen tvdijen force-pushed the Xnew-ui branch from 875da44 to fad3328 Aug 1, 2018

@tvdijen tvdijen merged commit fad3328 into master Aug 1, 2018

3 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 30.56%
Details
@tvdijen

This comment has been minimized.

Member

tvdijen commented Aug 1, 2018

Okay, I have no idea how this merge happened :/
I guess it's not a bad thing now the issue has been resolved... It's a bit radical though.. There may be glitches in master right now

@lonoak

This comment has been minimized.

Collaborator

lonoak commented Aug 1, 2018

Thanks for your efforts, Tim!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment