Discussion:
[Freeipa-devel] [freeipa PR#782][opened] [WIP] Improving GUI text in "Add DNS Zones" popup
felipevolpone
2017-05-11 23:42:12 UTC
Permalink
URL: https://github.com/freeipa/freeipa/pull/782
Author: felipevolpone
Title: #782: [WIP] Improving GUI text in "Add DNS Zones" popup
Action: opened

PR body:
"""
Improving usability of the "Add DNS Zones" popup in Web UI.

Ticket: https://pagure.io/freeipa/issue/6687
"""

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/782/head:pr782
git checkout pr782
pvoborni
2017-05-12 13:49:02 UTC
Permalink
URL: https://github.com/freeipa/freeipa/pull/782
Title: #782: [WIP] Improving GUI text in "Add DNS Zones" popup

pvoborni commented:
"""
I'm not completely sure that the approach suggested in bug report is correct. That is why I suggested alternative in https://bugzilla.redhat.com/show_bug.cgi?id=1419834#c2

So before implementing it a small conversation could have happen to agree on the approach.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/782#issuecomment-301081271
pvomacka
2017-05-12 15:46:09 UTC
Permalink
URL: https://github.com/freeipa/freeipa/pull/782
Title: #782: [WIP] Improving GUI text in "Add DNS Zones" popup

pvomacka commented:
"""
Hello @felipevolpone ,
Thank you for your patch.
For adding arbitrary text into a dialog or details page is probably the most suitable IPA.html_widget (but it has a big disadvantage - described below in section A). You can put it into the section you created. It might look like this:
```
{
name: 'dnszone_title',
show_header: false,
fields: [
{
field: false,
$type: 'html',
name: 'info',
html: "Select the required zone type."
}
],
layout: {
$factory: IPA.fluid_layout,
widget_cls: "col-sm-12 controls",
label_cls: "hide"
}
},
```
Layout attribute of the section might not be needed, but I would say that here it good to add it. It hides label of field and set width of the field to 100% of the dialog.

(Simpler solutions below - B and C)
A) The html attribute contains text which will be displayed. Text there should be taken from translatable strings. It can be done by using `text.get('i18n:path.to.the.string')` and writing the string into ipaserver/internal.py. The main challenge here might be to find a place where the string has to be loaded. It has to be done before building the whole dialog and its sections. You will probably need to override `dialog_build_properites` attribute of entity specification and there change `$post_ops` operation which where is the function which builds adder dialog for entity (add there loading of translate string).

B) (not tested) Another solution would be to set text field instead of html one and turn off the field in the same way as above and then set it non-writable and read_only. Then hide the label and there the `text.get()` should work directly in field definition. (should not be needed to change behavior of building entity's adder dialog).

C) Another solution will be to create new widget, which will work in the same way as `IPA.html_widget` but it will support translatable strings.

If you have any question feel free to ask. :)
"""

See the full comment at https://github.com/freeipa/freeipa/pull/782#issuecomment-301113031
pvomacka
2017-05-12 16:03:19 UTC
Permalink
URL: https://github.com/freeipa/freeipa/pull/782
Title: #782: [WIP] Improving GUI text in "Add DNS Zones" popup

pvomacka commented:
"""
Sorry I haven't refresh the page so I didn't see @pvoborni comment before I sent mine. The suggestion which Petr wrote into Bugzilla should be discussed with @MartinBasti and if I recall correctly he did not recommend it from point of view of DNS.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/782#issuecomment-301117428
pvoborni
2017-05-15 08:19:29 UTC
Permalink
URL: https://github.com/freeipa/freeipa/pull/782
Title: #782: [WIP] Improving GUI text in "Add DNS Zones" popup

pvoborni commented:
"""
Ok, when one field is not usuable because IP address or network address are also valid DNS zones, then the proper way is to follow patternfly design for this kind of workflows: http://www.patternfly.org/pattern-library/forms-and-controls/progressive-disclosure/
"""

See the full comment at https://github.com/freeipa/freeipa/pull/782#issuecomment-301408101
pvomacka
2017-05-15 11:37:41 UTC
Permalink
URL: https://github.com/freeipa/freeipa/pull/782
Title: #782: [WIP] Improving GUI text in "Add DNS Zones" popup

pvomacka commented:
"""
Yes, this pattern should be used. We already have a widget for this (without hiding not-selected area) and it is used i.e. in certmapdata adder dialog which could be opened from user's details page. Try to look for `multiple_choice_section`.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/782#issuecomment-301450276
felipevolpone
2017-05-19 20:26:31 UTC
Permalink
URL: https://github.com/freeipa/freeipa/pull/782
Title: #782: [WIP] Improving GUI text in "Add DNS Zones" popup

felipevolpone commented:
"""
I'm dropping this PR because I'm focused on other tickets for now
"""

See the full comment at https://github.com/freeipa/freeipa/pull/782#issuecomment-302803135
felipevolpone
2017-05-19 20:26:33 UTC
Permalink
URL: https://github.com/freeipa/freeipa/pull/782
Author: felipevolpone
Title: #782: [WIP] Improving GUI text in "Add DNS Zones" popup
Action: closed

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/782/head:pr782
git checkout pr782

Loading...