4:44 p.m.
----- Original Message -----
From: "Bill Burke" <bburke(a)redhat.com>
To: keycloak-dev(a)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(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/keycloak-dev