HUE-7873 [core] Update the AutoComplete for Document share in /home page

Review Request #12393 — Created Jan. 22, 2018 and submitted

roohi
hue
HUE-7774-Autocomplete
HUE-7873
hue
enricoberti, jgauthier, johan, ranade, romain, weixia, yingc
commit 20a00a661ac6471c9d7e6b6b799cb382abba2925
Author: Roohi <roohisyeda@cloudera.com>
Date:   Mon Jan 22 15:36:46 2018 -0800

    HUE-7873 [core] Update the AutoComplete for Document share in /home page

:100644 100644 209bd902b4... 762f324bb1... M	desktop/core/src/desktop/static/desktop/css/hue-embedded.css
:100644 100644 952b663103... e2aab9c253... M	desktop/core/src/desktop/static/desktop/css/hue.css
:100644 100644 d31882c347... 5399fcef73... M	desktop/core/src/desktop/static/desktop/css/hue3-extra.css
:100644 100644 b6d6e8e1a8... 1ef1b8143a... M	desktop/core/src/desktop/static/desktop/js/document/hueDocument.js
:100644 100644 5e1b4763f5... af27ccd347... M	desktop/core/src/desktop/static/desktop/js/ko.hue-bindings.js
:100644 100644 19bc740ab9... b38f5bb377... M	desktop/core/src/desktop/static/desktop/js/share2.vm.js
:100644 100644 6718570f08... d3edec8932... M	desktop/core/src/desktop/static/desktop/less/hue-cross-version.less
:100644 100644 cb869c3c02... 587d70adf1... M	desktop/core/src/desktop/templates/common_share.mako
:100644 100644 76c87b8e81... 77ec6952c2... M	desktop/core/src/desktop/templates/document_browser.mako


  • 7
  • 0
  • 20
  • 0
  • 27
Description From Last Updated
Why do we need this? the user object should already have a prettyName attribute after prettifyUserNames has been called johan johan
No need for return as the user object gets prettyName set johan johan
Why is this one needed? #ascrail2006 seems generated and a bit random johan johan
Do you have a screenshot of how this looks? johan johan
hueUtils.highlight johan johan
space after if: if (search... For consistency we always use brackets i.e. if (...) { return text; } johan johan
Nit: remove whitespace johan johan
roohi
  1. 
      
  2. I have an issue with autocomplete widget, when I select an item from autocomplete options, the modal fades and I can't see the modal, please see this https://drive.google.com/file/d/1xpquDX_-KBNQCqYfaJVroHSD7IXSNiTE/view?usp=sharing

    I have pushed my code to HUE-7873_Share_home

  3. 
      
roohi
roohi
  1. 
      
  2. The scroll is not visible on autocomplete drop down menu and the height increases as the size of drop down items increase. Enrico/Johan can you please check?

    1. Likely caused by some missing styles, height, max-height, overflow-y etc. Easiest to check the styles of a dropdown where it behave correctly, see if heights are specified and from where, then compare with this one.

    2. Thanks Johan. This helped while adding another scroll bar, but for the scroll bar for the ui autocomplete menu, I had to call resize()

    3. If you want you can push again to the branch after fixing the latest things Johan found, so we can quickly check about the scrollbar. Most of the times it depends on where the 'position: relative' is in all the parent elements :)

    4. Thanks Enrico. I have pushed to HUE-7873_Share_home_scroll

  3. 
      
roohi
johan
  1. 
      
  2. Camel case -> highlightedLabel

  3. onEnter is a bit too generic for hueDocument that is used elsewhere

  4. All caps = constant would expect immutable, missing var

  5. Same as above.

    Cleaner with function i.e. var highlighted = highlight(request.term)

  6. Safer to first check if it's there. I.e. the data and getNiceScroll()

  7. A bit risky to use regex as it'll break on any regex operators in request.term, more work but safer to do indexOf-based replace.

  8. 
      
roohi
roohi
johan
  1. 
      
  2. autocompleteSource needs to be more specific. I.e. for which autocomplete? Considering that this represents a Hue document.

  3. Should ignore case. Could be worth while adding to hue.utils.js

  4. Why do we need this? the user object should already have a prettyName attribute after prettifyUserNames has been called

    1. We need this for the users with whom the document has already been shared. That API only return ids not the Users with firstname & lastname

  5. No need for return as the user object gets prettyName set

  6. Why is this one needed? #ascrail2006 seems generated and a bit random

  7. This doesn't take case into account

  8. !important needed? Also better to lower it to just above where you need it to be.

  9. Do you have a screenshot of how this looks?

    1. Here it is https://drive.google.com/file/d/1YDmZfbVJdRVYNTLO2WGqrAetvpck_Njz/view?usp=sharing

  10. Perhaps remove the z-index above from the less as inline styles are used

  11. ...('read'); onEnter(); } should be enough

  12. 
      
roohi
johan
  1. Ship It!
  2. space after if: if (search...

    For consistency we always use brackets i.e.

    if (...) {
    return text;
    }

  3. Nit: remove whitespace

  4. 
      
roohi
Review request changed

Status: Closed (submitted)

Loading...