Details

    • Type: Improvement
    • Status: Closed
    • Priority: Trivial
    • Resolution: Implemented
    • Affects Version/s: Trunk
    • Fix Version/s: 16.11.01
    • Component/s: ecommerce
    • Labels:
      None

      Description

      Problem
      =======
      Go to https://localhost:8443/ecommerce/
      Popup (tooltip) contains the same image as the target.
      Pointless to repeat the same content in a popup (tooltip).

      1. OFBIZ-6968.patch
        2 kB
        James Yong
      2. OFBIZ-6968.patch
        2 kB
        James Yong
      3. beforeAndAfter.jpg
        55 kB
        James Yong

        Activity

        Hide
        jamesyong James Yong added a comment -

        This patch removes the duplicate picture from the popup.
        Also improves on the text alignment and style.
        Also fix cases where popup is missing when the target is near the browser bottom.

        Show
        jamesyong James Yong added a comment - This patch removes the duplicate picture from the popup. Also improves on the text alignment and style. Also fix cases where popup is missing when the target is near the browser bottom.
        Hide
        jamesyong James Yong added a comment -

        I need more work on it and will submit another patch.

        Show
        jamesyong James Yong added a comment - I need more work on it and will submit another patch.
        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        Hi James,

        This sounds good me but what if the end users want to use different images?

        Of course she has then to use something else than smallImageUrl in the "productDetailId" div, but that's possible. With your solution this possibility dissapears.
        Please note that the OOTB UI is mostly for demonstration purpose and is intented to be adapted by users for their need. It's more a template in a way...

        What others think about that before I close as "Won't Fix"?

        Show
        jacques.le.roux Jacques Le Roux added a comment - Hi James, This sounds good me but what if the end users want to use different images? Of course she has then to use something else than smallImageUrl in the "productDetailId" div, but that's possible. With your solution this possibility dissapears. Please note that the OOTB UI is mostly for demonstration purpose and is intented to be adapted by users for their need. It's more a template in a way... What others think about that before I close as "Won't Fix"?
        Hide
        pfm.smits Pierre Smits added a comment - - edited

        IMO we should apply this patch. Even though it is only used in a 'demo' setup, it will give an impression. And it is better to provide a simple impression than an annoying one. Even if it is of minor importance. Code cleanliness also leaves an impression.

        Show
        pfm.smits Pierre Smits added a comment - - edited IMO we should apply this patch. Even though it is only used in a 'demo' setup, it will give an impression. And it is better to provide a simple impression than an annoying one. Even if it is of minor importance. Code cleanliness also leaves an impression.
        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        Thanks Pierre,

        This was introdiced with OFBIZ-1891. I'd like to have at least the opinion of the people who created it Sumit Pandit and Rishi Solanki or related (IIRW they were then working for Hotwax Media now Hotwax System).

        Personaly, what I'd like to do is to use James's code but to keep the image but using LARGE_IMAGE_URL instead, opinion?

        Show
        jacques.le.roux Jacques Le Roux added a comment - Thanks Pierre, This was introdiced with OFBIZ-1891 . I'd like to have at least the opinion of the people who created it Sumit Pandit and Rishi Solanki or related (IIRW they were then working for Hotwax Media now Hotwax System). Personaly, what I'd like to do is to use James's code but to keep the image but using LARGE_IMAGE_URL instead, opinion?
        Hide
        pfm.smits Pierre Smits added a comment -

        Let us wait and see whether there will be a reaction.

        Show
        pfm.smits Pierre Smits added a comment - Let us wait and see whether there will be a reaction.
        Hide
        pfm.smits Pierre Smits added a comment -

        You can apply lazy-consensus here too. It doesn't break stuff.

        Show
        pfm.smits Pierre Smits added a comment - You can apply lazy-consensus here too. It doesn't break stuff.
        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        Thanks James, Pierre,

        James, your modified patch is committed at revision: 1737627

        I added the introduction of LARGE_IMAGE_URL and updated to reflect the productsummary.ftl location change

        Show
        jacques.le.roux Jacques Le Roux added a comment - Thanks James, Pierre, James, your modified patch is committed at revision: 1737627 I added the introduction of LARGE_IMAGE_URL and updated to reflect the productsummary.ftl location change

          People

          • Assignee:
            jacques.le.roux Jacques Le Roux
            Reporter:
            jamesyong James Yong
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development