[keycloak-dev] Code and console inspection

Viliam Rockai vrockai at redhat.com
Tue Nov 19 08:23:12 EST 2013


Hi all,

So, I've looked at both - the code and the console UI. I've created 
bunch of JIRAs today and I still have some unclear issues. Here's my list:

  * What's the point of admin-ui-styles module? Why does it contain html?
      o I had troubles with this when I was working on the social page
        and some (provider helper) pages were used from this module
        instead of admin-ui module.

  * Directive naming not unified (ie. uncollapsed vs. kcInput).
      o Do we care about that (it's angularJS good practice)? I think it
        would be not just correct but even pretty to have kc prefix
        everywhere.

  * I see combined AngularJS + JQuery code on some places.
      o AFAIK this should be avoided unless there is no angular way how
        to solve the problem.

  * Array.prototype.remove from controller.js used only on one (mine)
    place, on other places we use .splice to remove from array.
      o Why was this method created? What's the advantage when compared
        to splice?

  * Lots of warnings in the JS code (like herd of unused $q in loaders.js).

  * They way the menu (<ul class="rcue-tabs">) / breadcrums is generated
    (hardcoded on each page) results in big amount of redundant code
    (this bothers me a lot).
      o One of the actual consequences of this is - if you traverse
        through realm settings for most pages the "settings" line/tab on
        the left is highlighted, but for some (registration, keys) it's not.
      o If you add new page, you have to update the menu on many other
        pages and you can easily miss some as in the situation above.

  * Missing l18n.

  * Credentials page for user is missing.

  * Session pages for "Applications" part are missing.

  * Manage account of the logged-in user is missing.
      o Clicking on the "manage account" in logged-in user menu (top
        right) does nothing.

And the nitpicker bonus:

  * What about using the maven-checkstyle-plugin? :)

Thanks for your thoughts on this,

Viliam

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/keycloak-dev/attachments/20131119/8029898f/attachment.html 


More information about the keycloak-dev mailing list