[keycloak-dev] Code and console inspection

[Stian Thorgersen]

----- Original Message -----
> From: "Bill Burke" <bburke at redhat.com>
> To: keycloak-dev at lists.jboss.org
> Sent: Tuesday, 19 November, 2013 1:34:42 PM
> Subject: Re: [keycloak-dev] Code and console inspection
> 
> 
> 
> On 11/19/2013 8:23 AM, Viliam Rockai wrote:
> > 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.
> >
> 
> Could be merged with admin-ui module.  This is just an artifact of how
> the project evolved.

+1 We should have a common-styles module instead. It should only contain stylesheets/images/etc that are used by both admin and forms.

> 
> >   * 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.
> >
> 
> The session timeout logic is JQuery.  If you know an Angular way, feel
> free to change it.

Just wrap it in a directive. I believe you should never ever manipulate the DOM outside of directives. This because you're doing changes to the DOM that Angular is unaware of which can cause weird side-effects.

Villiam this would be a good exercise if you haven't already written a directive ;)

> 
> >   * 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?
> >
> 
> A lot of this is that some Javascript noob wasn't sure how to do
> something in Javascript so he cut and pasted from Stackoverflow.  I
> wonder who that was?  :)
> 
> >   * 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.
> >
> 
> Laziness by the Javascript noob again.
> 
> >   * Missing l18n.
> >
> 
> This will be a bigger task as we'll want to configure how things like
> Grant requests and their roles are displayed.  A lot of things like that
> 
> >   * Credentials page for user is missing.
> >
> 
> I think there is a JIRA for this IIRC.  This is a placeholder page to be
> able to reset credentials when a user forgets password or you want to
> force a credential change.

There is, and it's assigned to Villiam I believe ;)

> 
> >   * Session pages for "Applications" part are missing.
> >
> 
> Another placeholder.  This will allow the admin to query or see which
> users have sessions on an application so that they can be automatically
> logged out.

We should comment the link out for now, as I assume this is not something we'll implement for M1?

> 
> >   * 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? :)
> >
> 
> Sure.  I don't care.
> 
> --
> Bill Burke
> JBoss, a division of Red Hat
> http://bill.burkecentral.com
> _______________________________________________
> keycloak-dev mailing list
> keycloak-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/keycloak-dev
>