[Freeipa-devel] [PATCH] 463-530 First part of RCUE adoption

[Martin Kosek]

On 01/16/2014 12:06 PM, Petr Vobornik wrote:
> On 3.1.2014 12:38, Petr Vobornik wrote:
>> On 29.11.2013 17:02, Ana Krivokapic wrote:
>>> On 11/29/2013 02:45 PM, Ana Krivokapic wrote:
>>>> >On 11/25/2013 04:48 PM, Ana Krivokapic wrote:
>>>>> >>On 11/15/2013 02:26 PM, Petr Vobornik wrote:
>>>>>> >>>Hello list,
>>>>>> >>>
>>>>>> >>>this is a first part of RCUE adoption effort. Main themes of
>>>>>> this patch set are:
>>>>>> >>>
>>>>>> >>>- use RCUE navigation<https://fedorahosted.org/freeipa/ticket/3902>
>>>>>> >>>- new styles for textboxes, textareas, radio/checkbox buttons
>>>>>> and buttons-
>>>>>> >>>part of<https://fedorahosted.org/freeipa/ticket/3904>
>>>>>> >>>- new internal form layout (tables replaced by divs)
>>>>>> >>>- layout does not have fixed
>>>>>> size<https://fedorahosted.org/freeipa/ticket/3435>
>>>>>> >>>- new dialog styles + removed dependency on jQuery UI dialog
>>>>>> >>>- icons replaced by Font Awesome glyphs
>>>>>> >>>
>>>>>> >>>Example is at:<http://pvoborni.fedorapeople.org/rcue/>
>>>>>> >>>
>>>>>> >>>Some reasonings and additional info:
>>>>>> >>>
>>>>>> >>>1. RCUE includes Bootstrap which defines o lot of styles for a
>>>>>> lot of things.
>>>>>> >>>That messed up the UI and therefore I did the form changes now.
>>>>>> >>>
>>>>>> >>>2. jQuery UI is pretty big lib and we used it only for dialog
>>>>>> and buttons.
>>>>>> >>>Buttons were replaced by RCUE buttons so removal of dialog
>>>>>> dependency was a
>>>>>> >>>obvious step to get rid of the whole lib. The lib is removed
>>>>>> from main UI but
>>>>>> >>>is still present in separate pages - will be removed later.
>>>>>> >>>
>>>>>> >>>3. Dojo and jQuery were upgraded to latest
>>>>>> >>>versions.<https://fedorahosted.org/freeipa/ticket/2811>
>>>>>> >>>
>>>>>> >>>This approach was ACKed by Kyle from a design perspective with a
>>>>>> note that we
>>>>>> >>>will review and fixed some styling after second phase. We should
>>>>>> not release
>>>>>> >>>until then.
>>>>>> >>>
>>>>>> >>>The second phase, which I'm working on right now, will consist of:
>>>>>> >>>  * login screen<https://fedorahosted.org/freeipa/ticket/3903>
>>>>>> >>>  * new styles for standalone pages
>>>>>> >>>  * necessary responsive enhancement (the ultimate future goal
>>>>>> is responsive
>>>>>> >>>layout)
>>>>>> >>>
>>>>>> >>>It's quite a lot of patches so I did not attach them here. You
>>>>>> can see the
>>>>>> >>>code in my private repo:
>>>>>> <git://fedorapeople.org/~pvoborni/freeipa.git> branch
>>>>>> >>>'rcue'.
>>>>> >>I tested this phase of RCUE adoption effort, overall it looks and
>>>>> works great. A
>>>>> >>couple of findings:
>>>>> >>
>>>>> >>1) Two ui integration tests are failing, I guess this is due to
>>>>> the re-arranging
>>>>> >>of elements on the automember page. So the tests should be amended
>>>>> to reflect
>>>>> >>that change.
>>> Commit ee4f6540490a16f0fbb5cdd02097a9b3ff354252 works around the
>>> navigation
>>> issue which caused tests to fail. ACK.
>>>
>>
>> I've unbundled font files from FreeIPA Web UI
>> - Open Sans will be in new fedora package - open-sans-fonts
>> - Font Awesome as well - fontawesome-fonts
>> - Overpass fonts are no longer used
>>
>> Branch rcue-fonts-unbundled
>> git://fedorapeople.org/home/fedora/pvoborni/public_git/freeipa.git
>> contains the changes. It's 6 additional commits on top of the reviewed
>> rcue branch. 5 commits were squashed in order to reduce size of the
>> final push. Attaching all of the patches for reference.
>>
>> open-sans-fonts and fontawesome-fonts Fedora packages are still on review:
>> - https://bugzilla.redhat.com/show_bug.cgi?id=1035897
>> - https://bugzilla.redhat.com/show_bug.cgi?id=1026376
>>
>> Reviewer for open-sans-fonts needed.
>>
>> OTP UI will be rebased on top of this effort.
>>
> 
> The review is not blocked anymore - both fonts are available in updates testing.

I tested the 'rcue-fonts-unbundled' branch and it works for me. Given Ana's
previous review, the only unreviewed patch is the top one switching to using
the external fonts you packaged [1]. I found just one issue in the patch - the
version of ipa.conf was not bumped, thus the upgrade would fail as ipa.conf
would not be updated.

If this is fixed, I would be OK with pushing this work to master so that you
can base your next work on a stable grounds. Adding Simo and Rob to CC to
notify them about this bigger push so that they can express any concerns.

Note that current state is still not ready to be released as is, it still has
some rough edges, but it will at least give others developers opportunity to
test it.

Martin

[1]
http://fedorapeople.org/cgit/pvoborni/public_git/freeipa.git/commit/?h=datetime-ui&id=3d976cdb60e122ea722ecb3bd03a78b25ef105ca