7:23 a.m.
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
7:34 a.m.
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.
* 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.
* 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.
* 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.
* 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
9:44 a.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
8:16 a.m.
On 11/19/2013 10:44 AM, Stian Thorgersen wrote:
----- 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.
Please let me know what happens. My dev setup has a bunch of softlinks
into a JBoss distro for fast turnaround. Refactoring this would break
things.
>
>> * 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 ;)
I tiny writeup of what you do to correct this would be great.
>
> 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?
Under construction image instead?
--
Bill Burke
JBoss, a division of Red Hat
http://bill.burkecentral.com
11:02 a.m.
----- Original Message -----
From: "Bill Burke" <bburke(a)redhat.com>
To: "Stian Thorgersen" <stian(a)redhat.com>
Cc: keycloak-dev(a)lists.jboss.org
Sent: Wednesday, 20 November, 2013 2:16:49 PM
Subject: Re: [keycloak-dev] Code and console inspection
On 11/19/2013 10:44 AM, Stian Thorgersen wrote:
>
>
> ----- 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.
>
Please let me know what happens. My dev setup has a bunch of softlinks
into a JBoss distro for fast turnaround. Refactoring this would break
things.
Why don't you just use:
# mvn -pl testsuite/integration -Pkeycloak-server -Dresources exec:java
That loads the resources directly from the file-system so no need for softlinks, or JBoss
;)
>>
>>> * 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 ;)
>
I tiny writeup of what you do to correct this would be great.
>>
>> 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?
>
Under construction image instead?
--
Bill Burke
JBoss, a division of Red Hat
http://bill.burkecentral.com
12:40 p.m.
On 11/20/2013 12:02 PM, Stian Thorgersen wrote:
----- Original Message -----
> From: "Bill Burke" <bburke(a)redhat.com>
> To: "Stian Thorgersen" <stian(a)redhat.com>
> Cc: keycloak-dev(a)lists.jboss.org
> Sent: Wednesday, 20 November, 2013 2:16:49 PM
> Subject: Re: [keycloak-dev] Code and console inspection
>
>
>
> On 11/19/2013 10:44 AM, Stian Thorgersen wrote:
>>
>>
>> ----- 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.
>>
>
> Please let me know what happens. My dev setup has a bunch of softlinks
> into a JBoss distro for fast turnaround. Refactoring this would break
> things.
Why don't you just use:
# mvn -pl testsuite/integration -Pkeycloak-server -Dresources exec:java
That loads the resources directly from the file-system so no need for softlinks, or JBoss
;)
Oh, cool, thought that ran from web-fragment jars.
--
Bill Burke
JBoss, a division of Red Hat
http://bill.burkecentral.com
12:50 p.m.
With '-Dresources' it uses the files directly, without it, it uses the classpath.
So remember '-Dresources' ;)
----- Original Message -----
From: "Bill Burke" <bburke(a)redhat.com>
To: "Stian Thorgersen" <stian(a)redhat.com>
Cc: keycloak-dev(a)lists.jboss.org
Sent: Wednesday, 20 November, 2013 6:40:26 PM
Subject: Re: [keycloak-dev] Code and console inspection
On 11/20/2013 12:02 PM, Stian Thorgersen wrote:
>
>
> ----- Original Message -----
>> From: "Bill Burke" <bburke(a)redhat.com>
>> To: "Stian Thorgersen" <stian(a)redhat.com>
>> Cc: keycloak-dev(a)lists.jboss.org
>> Sent: Wednesday, 20 November, 2013 2:16:49 PM
>> Subject: Re: [keycloak-dev] Code and console inspection
>>
>>
>>
>> On 11/19/2013 10:44 AM, Stian Thorgersen wrote:
>>>
>>>
>>> ----- 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.
>>>
>>
>> Please let me know what happens. My dev setup has a bunch of softlinks
>> into a JBoss distro for fast turnaround. Refactoring this would break
>> things.
>
> Why don't you just use:
>
> # mvn -pl testsuite/integration -Pkeycloak-server -Dresources exec:java
>
> That loads the resources directly from the file-system so no need for
> softlinks, or JBoss ;)
>
Oh, cool, thought that ran from web-fragment jars.
--
Bill Burke
JBoss, a division of Red Hat
http://bill.burkecentral.com
4072
days inactive
4073
days old
6 comments
3 participants
participants (3)
-
Bill Burke -
Stian Thorgersen -
Viliam Rockai