[Freeipa-devel] [PATCH] 592-628 Update to PatternFly

[Petr Vobornik]

On 6.6.2014 20:35, Endi Sukma Dewata wrote:
> On 6/6/2014 10:43 AM, Petr Vobornik wrote:
>> On 6.6.2014 15:45, Endi Sukma Dewata wrote:
>>> On 6/5/2014 9:25 AM, Endi Sukma Dewata wrote:
>>>> ACK for patches #592-#628. I'll continue reviewing the rest.
>>>
>>> ACK for patches #633-639, #642, #644, #652, and #653. Patches #640 &
>>> #641 have an issue (see #19 below) that should be fixed before pushing.
>>> Other issues are minor/unrelated/suggestions that can be addressed
>>> separately.
>>
>> Thank you for the review.
>>
>> I've fixed issues:
>>
>> - #19, #20 in patch  #640.
>>
>> And some low hanging fruit:
>> - #16, in patch #637
>> - #17, in patch #612
>> - #13, in patch #612
>>
>> The branch has been rebased to current master.
>
> ACK.

I've fixed issues #4, #2, #20 and #18. Commits in the branch, no rebase.

With these 4 changes we are ready for the push. I'll squash them, if 
necessary.


>
>> I am not able to reproduce issues #4 and #11.
>
>>> 4. In the list page (e.g. Users) in mobile mode the Refresh button
>>> may overlap the search box.
>
> Here's what I saw as I was adjusting the page width:
> http://edewata.fedorapeople.org/ipa/images/snapshot1.png
> http://edewata.fedorapeople.org/ipa/images/snapshot2.png
> http://edewata.fedorapeople.org/ipa/images/snapshot3.png
>
> Notice that in snapshot #2 the search box is partially covered by the
> Refresh button.
>
>>> 11. In desktop mode the QR code for new OTP token is displayed
>>> outside the dialog box.
>
> Here's what I saw:
> http://edewata.fedorapeople.org/ipa/images/snapshot4.png

Fixed, both were Firefox-only issues.

>
>>> 18. In the New Certificate dialog for Host, the instruction to create a
>>> CSR exceeds the dialog boundary.
>>
>> caused by BS' CSS:
>> code {
>>     white-space: nowrap;
>> }
>>
>> I wonder if the best solution is to reset it to initial value in all
>> dialogs.
>
> Alternatively, the sample command could be broken into two lines.

IMHO the style is better, because it's more general and easier for 
copy&paste. I've also added word-wrap to break very long subjects.

>
>>> 20. The capitalization of "Certificate" is inconsistent in Host's and
>>> Service's Actions.
>>
>> Fixed
>
> The "View certificate" is still inconsistent though.
>

Shame on me :)

-- 
Petr Vobornik