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

Fix new UI #906

Merged
merged 66 commits into from Oct 5, 2018

Conversation

@tvdijen
Member

tvdijen commented Aug 6, 2018

Last week, the Xnew-ui branch was accidentally merged into master. There are however still a couple of issues that need to be taken care of. I will keep track of the remaining issues here, making this PR the successor of #717. Help is very much appreciated!

The aim here is to make master working properly ASAP.

Tagging previous contributors: @Olimpiamagliulo, @thijskh, @dnmvisser, @pmeulen, @joostd

@tvdijen tvdijen added this to the 1.17 milestone Aug 6, 2018

@tvdijen

This comment has been minimized.

Member

tvdijen commented Aug 6, 2018

Checklist (not limited to *):

- [x] consent: module stylesheet is not loaded Was fixed in f8e6bac

- [x] consent: pressing the 'yes' or 'no' button leads to a broken redirect Was fixed in 3b2056f

- [x] statistics: showing a proper error when no data is available was introduced in dd0a0af, but the Twig-template still looks horrible when this happens.. Need to mimic the non-Twig behaviour Was fixed in 2a3785d

- [x] statistics: Twig-template contains JS that should be moved to a separate file Was fixed in 2a3785d

- [x] consentAdmin: if you try to open the consentAdmin-page when you've never given consent for a service, an 'unhandled exception' is raised. This also happens when withdrawing the last consent from your overview. Withdrawing consent leads to a broken/messed-up page anyway, even using the old templates. update: same happens when given consent is older than x hours? Not sure
Was fixed in 20e1ca4 (master)

- [x] consentAdmin: the templates contain inline javascript was fixed in d4d9102

- [x] negotiate: still has on the fly composed javascript in lib/Auth/Source/Negotiate.php (sendNegotiate). Was fixed in 8d181c0

- [x] authYubiKey: old templates don't work anymore, autofocus fails on Twig-templates Was fixed in e3479eb

- [x] autofocus doesn't work on Twig-templates when $this->data['autofocus'] is defined. Was fixed in 4b96f74

  • Old style templates do not work any more, since the new Twig-templates take precedence over the old ones and language.i18n.backend => 'SimpleSAMLphp' is therefore not doing what it's supposed to do.
    See Jaime's comment on this issue here: #905 (comment)

  • add documentation on currently undocumented theme.controller-setting

  • consent: consent-form still looks like 💩 on a cracker and needs work

  • NOSTATE errors are not translated properly
    afbeelding

  • Metadata not found are errors not translated properly
    afbeelding

  • Modules that have hooks on the config frontpage need a translation in core/locales... Right now, the hooks for statistics, sanitycheck and memcachemonitor (list not limited) are broken. I think this undermines the 'hooks'-model.. Any module should be able to hook into the config-page.

  • discopower: still has on the fly composed javascript in lib/DiscoPower.php (handleRequest)

  • There is no way to logoff from your admin dashboard.

  • The menu of the admin dashboard doesn't properly show what tab is selected. It does switch classes properly and the selected item is bold, but the underlining is always looking like the 'Federation'-tab is selected.

Leftovers from #717 for Twigification:
- [x] consentAdmin (css/js already moved to assets) was added in 01d42fe

  • core/templates/logout-iframe.php

*) If you have any issue to add here, let me know!

@tvdijen tvdijen self-assigned this Aug 6, 2018

@tvdijen tvdijen force-pushed the Xnew-ui-fixes branch from 5248936 to 36d1861 Aug 9, 2018

tvdijen added some commits Aug 9, 2018

@tvdijen tvdijen force-pushed the Xnew-ui-fixes branch 8 times, most recently from d8efbaf to be5829b Aug 10, 2018

@tvdijen tvdijen force-pushed the Xnew-ui-fixes branch from c5b460d to fabe0dd Aug 15, 2018

@tvdijen tvdijen force-pushed the Xnew-ui-fixes branch from fabe0dd to f4d81c1 Aug 15, 2018

tvdijen added some commits Aug 15, 2018

@tvdijen tvdijen force-pushed the Xnew-ui-fixes branch 4 times, most recently from 3bdb361 to fcb2358 Aug 25, 2018

@tvdijen tvdijen force-pushed the Xnew-ui-fixes branch from fcb2358 to 668836b Aug 26, 2018

@jaimeperez

This comment has been minimized.

Member

jaimeperez commented Sep 27, 2018

  • Old style templates do not work any more, since the new Twig-templates take precedence over the old ones and language.i18n.backend => 'SimpleSAMLphp' is therefore not doing what it's supposed to do.
    See Jaime's comment on this issue here: #905 (comment)

See this comment as well.

  • add documentation on currently undocumented theme.controller-setting

This was introduced to allow modules to perform certain tasks when displaying pages. I can probably try to document it, although I think it really belongs together with the documentation on custom theming.

  • Modules that have hooks on the config frontpage need a translation in core/locales... Right now, the hooks for statistics, sanitycheck and memcachemonitor (list not limited) are broken. I think this undermines the 'hooks'-model.. Any module should be able to hook into the config-page.

Fully agree. We need to find a solution to this. Moving away from pure-PHP templates makes it harder to run hooks. We need to investigate whether twig provides a way to offer some similar functionality.

I think we also need to work on a proper deployment mechanism using webpack or something like that. I'd move all source code (including CSS and JS) outside of www/, then have a build process that pulls the dependencies, packs everything together and dists the bundles to www/. This is essential to optimise the use of CSS and JavaScript, such as:

  • we have the optimal amount of JS/CSS files.
  • we can minimise bundles, making them as small as possible.
  • we can implement cache-busting mechanisms.
  • we can also implement proper content security policies, forbidding any inline JS.

Since I've been working a lot with all that stuff lately, that's also something I can probably work wit

Maybe we should start using projects again to organise what's needed and coordinate the work?

@jaimeperez jaimeperez merged commit 9a988ee into master Oct 5, 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 increased (+1.2%) to 31.709%
Details

@tvdijen tvdijen deleted the Xnew-ui-fixes branch Oct 5, 2018

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