OFBiz
  1. OFBiz
  2. OFBIZ-2507

Sort options in form fields by their visible description (also if localized)

    Details

    • Type: Improvement Improvement
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: SVN trunk
    • Fix Version/s: SVN trunk
    • Component/s: framework
    • Labels:
      None

      Description

      Currently, when options are shown in drop-down fields we can use entity-order-by to sort the options by e.g. description.

      But the order in which options appear is only valid for non-localized descriptions.

      If we have localized descriptions we want to make sure that these also appear in order, otherwise we make it difficult for the user to find the desired option especially in drop-down fields with many options.

      Btw i consider this major because for users who do not use the default language:

      • the presentation is non-standard and a usability obstacle
      • it's simply a real pain to look for an option in 'strangely-ordered' drop-downs

      Proposed Solution

      My proposal is to have a default ordering based on the description value of the OptionValue key-description-pairs in ModelFormField.java.

      The advantage of doing this is that you get what you'd normally expect, by default, in any drop-down field without even having to tell the system to order by description. An implementation should of course respect entity-order-by if present.

        Activity

        Hide
        Karim Rahimpur added a comment -

        This patch adapts ModelFormField.java implementing the proposed solution:

        • order options by the description that the user will see, if:
          1. no specific field(s) is to be used for sorting (<entity-order-by field-name="..."/>)
          2. the description field should be used to order by (<entity-order-by field-name="description"/>)

        This takes care of the problem and also, no changes to existing forms are needed with this approach (as there are drop-down fields that indicate the description field with entity-order-by).

        Show
        Karim Rahimpur added a comment - This patch adapts ModelFormField.java implementing the proposed solution: order options by the description that the user will see, if: 1. no specific field(s) is to be used for sorting (<entity-order-by field-name="..."/>) 2. the description field should be used to order by (<entity-order-by field-name="description"/>) This takes care of the problem and also, no changes to existing forms are needed with this approach (as there are drop-down fields that indicate the description field with entity-order-by).
        Hide
        Scott Gray added a comment -

        Hi Karim

        description is nothing more than an entity field name and the results would already have been sorted by that during the query if specified so there is no need to include it in your if condition. You should simply only sort if the entity-order-by attribute is false.

        Also you can use Collections.sort(optionValues, new Comparator() ...
        instead of putting everything into an array, sorting and pulling them back out again.

        Regards
        Scott

        Show
        Scott Gray added a comment - Hi Karim description is nothing more than an entity field name and the results would already have been sorted by that during the query if specified so there is no need to include it in your if condition. You should simply only sort if the entity-order-by attribute is false. Also you can use Collections.sort(optionValues, new Comparator() ... instead of putting everything into an array, sorting and pulling them back out again. Regards Scott
        Hide
        Karim Rahimpur added a comment -

        Hi Scott,

        Thanks for your comments, let me explain:

        • The reason for explicitly checking the description field and not only testing if the entity-order-by attribute is empty is that other fields than 'description' can be used to order the options (there really are, see e.g. InvoiceForms.xml the NewPurchaseInvoice form to name one of many cases) and we don't want to interfere with that. Now if we only check for entity-order-by being empty, we'll end up with what we had before: when entity-order-by is used to order by description, we won't get our correctly ordered localized list of options. So in order to also order by localized description we have to check if the entity-order-by field is 'description'.

        For storing the ordered list you could also use Collections.sort(...) directly if you prefer, although there would at least be no 'speed gain' as the ops are equivalent.

        Regards,
        Karim

        Show
        Karim Rahimpur added a comment - Hi Scott, Thanks for your comments, let me explain: The reason for explicitly checking the description field and not only testing if the entity-order-by attribute is empty is that other fields than 'description' can be used to order the options (there really are, see e.g. InvoiceForms.xml the NewPurchaseInvoice form to name one of many cases) and we don't want to interfere with that. Now if we only check for entity-order-by being empty, we'll end up with what we had before: when entity-order-by is used to order by description, we won't get our correctly ordered localized list of options. So in order to also order by localized description we have to check if the entity-order-by field is 'description'. For storing the ordered list you could also use Collections.sort(...) directly if you prefer, although there would at least be no 'speed gain' as the ops are equivalent. Regards, Karim
        Hide
        Scott Gray added a comment -

        I see what you are trying to do but at the end of the day description is still just an entity field to sort by, a field called "name" could be localized and your code wouldn't handle that situation, you are also enforcing the additional work of sorting when localization may not have taken place (although I guess it would be pretty quick considering the list would already be in the correct order). That said, while I don't think it is an ideal solution I do think it is acceptable. In an ideal world the GenericDelegator would be aware of the locale and also know that an order by field was localized and handle the localized sorting internally before we even see the list.

        Collections.sort(...) should be quicker because currently you are creating an array, iterating, sorting, clearing the list and iterating again whereas it could just be a sort only. But anyway cleaner code was the main reason for suggesting the change.

        Show
        Scott Gray added a comment - I see what you are trying to do but at the end of the day description is still just an entity field to sort by, a field called "name" could be localized and your code wouldn't handle that situation, you are also enforcing the additional work of sorting when localization may not have taken place (although I guess it would be pretty quick considering the list would already be in the correct order). That said, while I don't think it is an ideal solution I do think it is acceptable. In an ideal world the GenericDelegator would be aware of the locale and also know that an order by field was localized and handle the localized sorting internally before we even see the list. Collections.sort(...) should be quicker because currently you are creating an array, iterating, sorting, clearing the list and iterating again whereas it could just be a sort only. But anyway cleaner code was the main reason for suggesting the change.
        Hide
        Karim Rahimpur added a comment -

        a field called "name" could be localized and your code wouldn't handle that situation

        Yes that is true but my intent is to handle descriptions now because it's really an issue, the typical client's comment is 'why isn't the list in order? It's so hard to find the right option, ...'

        In an ideal world the GenericDelegator would be aware of the locale and also know that an order by field was localized and handle the localized sorting internally before we even see the list.

        That could be a good approach ...

        But anyway cleaner code was the main reason for suggesting the change.

        I now but as I was looking at the Collection.sort(...) code I just had to mention it ...

        Show
        Karim Rahimpur added a comment - a field called "name" could be localized and your code wouldn't handle that situation Yes that is true but my intent is to handle descriptions now because it's really an issue, the typical client's comment is 'why isn't the list in order? It's so hard to find the right option, ...' In an ideal world the GenericDelegator would be aware of the locale and also know that an order by field was localized and handle the localized sorting internally before we even see the list. That could be a good approach ... But anyway cleaner code was the main reason for suggesting the change. I now but as I was looking at the Collection.sort(...) code I just had to mention it ...
        Hide
        Karim Rahimpur added a comment -

        Oops ... of course I wanted to say 'I know ...'

        Btw on second thought about the GenericDelegator being aware of the locale, this class is about data but localizing data is about presentation so it's not where one should be aiming at to 'automate' localization. So taking that into account, I say the ModelFormField which is about presentation is the right place to automate localization in this case as proposed.

        Regards,
        Karim

        Show
        Karim Rahimpur added a comment - Oops ... of course I wanted to say 'I know ...' Btw on second thought about the GenericDelegator being aware of the locale, this class is about data but localizing data is about presentation so it's not where one should be aiming at to 'automate' localization. So taking that into account, I say the ModelFormField which is about presentation is the right place to automate localization in this case as proposed. Regards, Karim

          People

          • Assignee:
            Unassigned
            Reporter:
            Karim Rahimpur
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:

              Development