On 9/28/2011 7:59 PM, Adam Young wrote:
On 09/28/2011 06:50 PM, Endi Sukma Dewata wrote:
A new IPA.dialog_button class has been added to encapsulate the
buttons in the dialog box so they can be managed more easily.

The adder dialog has been modified to disable the enroll button if
there is no entries selected.

Ticket #1856

Pretty extensive change for such a small piece of functionailty. What is
the rationale?

The solution is simple, but it requires refactoring which probably should have been done much earlier. So this patch is like a combination of several patches.

When creating a jQuery dialog we only specify the button labels and their handlers. The dialog API doesn't give us access to the button's DOM elements. In order to disable a button, we have to find the button element within the dialog. However, these buttons do not have a name/id. We can search by the label, but it's not ideal. So the code is matching the button elements with button definitions based on their index.

To avoid repeating the search every time we need the button, the code is storing the DOM element in the button object. The button objects are stored in a map so they can be accessed by name.

Once we have the element, enabling/disabling the button is just a matter of adding/removing a CSS class. We also need to be able to check whether the button is enabled/disabled. These functionalities are encapsulated in IPA.dialog_button class.

For consistency with the rest of the code, all button definitions using add_button(label, handler) are converted into create_button(spec). This is where the majority of the changes happen. The changes could have been minimized by keeping a backward compatible interface, but it will clutter the code and eventually they are going to be replaced by the new interface anyway.

As a further enhancement, the IPA.dialog_button class probably can be combined with the IPA.action_button/IPA.button class so we can use icons or enable/disable buttons in a more consistent way. This is one step in that direction.

--
Endi S. Dewata

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to