Skip to content
This repository has been archived by the owner on Dec 13, 2020. It is now read-only.

Lookup revamp – collective issue #600

Closed
cadavre opened this issue Mar 31, 2017 · 25 comments
Closed

Lookup revamp – collective issue #600

cadavre opened this issue Mar 31, 2017 · 25 comments

Comments

@cadavre
Copy link
Contributor

cadavre commented Mar 31, 2017

I'm wrapping here issues: #161 #373 #531 #570 #568 . We're few long months right now after creating a vision of lookup – in meantime there was keyboard handling added to system, auto-filling with default values and more... This issue is a wrap of how it should finally look, taking into consideration nowadays assumptions.

Assumptions

  • Each field n+1 in fields array is considered as child to predecessors.
----------------------------
| field1 | field2 | field3 |
----------------------------
  ^parent  ^child   ^child

----------------------------
| field1 | field2 | field3 |
----------------------------
           ^parent  ^child

Focus on lookup while opening document

  1. If lookup is 1st widget in NEW document – focus on it allowing enter BPartner name.
  2. If lookup is 1st widget in EXISTING document (with already-filled BPartner) – focus on it, allow change BPartner name but don't display suggestions dropdown initially.

Navigation

Keyboard navigation

  • Pre-select first element in this dropdown and allow use [up]/[down] keys to browse through suggestions.
  • Allow [enter] key to accept suggestions dropdown selection.
  • [Tab] key allows to navigate only between widgets, not between fields inside of lookup. Pressing [tab] while inside of lookup changes focus to next widget.
  • If there are no default values for children – after selecting value in parent frontend is automatically showing suggestions (for list) or moving focus to typeahead (for suggestions) of a child. This way user can immediately select value for child for whom no default was provided.
  • If user wants to change child field which was filled by default – he uses mouse (because it's 5% of cases and we won't affect 95% of users with worse UX to solve it).

Mouse navigation

  • User can click on filled field to open suggestions dropdown or enter typing.
  • User can click first non-filled field to open suggestions dropdown or enter typing.
  • User cannot click non-first non-filled fields to edit. User can see "not allowed" cursor while trying to click.
  • User while hovering first non-editable can see field's background color. Color TBA

Clearing fields

  • User can click "x" at the end of lookup to clear all fields.
  • User can click on already-filled typeahead field. On click its whole value gets selected without possibility to change this selection (2nd click and [left]/[right] doesn't change selection and leave existing selection). User can press then [backspace] to remove value totally.
  • Pressing [backspace] clears field and all of its children!

"Mandatory" and "validation" borders

Lookup state

  • If any field have valid: false – lookup is considered as in valid: false state.
    [if (!field1.valid or !field2.valid or !field3.valid) then lookup.valid = false]
  • If any field have initialValue: true – lookup is considered as in initialValue: true state.
    [if (field1.initialValue or field2.initialValue or field3.initialValue) then lookup.initialValue = true]

Borders

  • Lookup initialValue: true gives blue border.
  • Border stays blue as long as frontend sends first PATCH.
  • Lookup initialValue: false and any mandatory field gets cleared (lookup gets valid: false) – border goes red.
  • Border stays red until lookup gets valid: true.

Layouting

  • All fields inside of lookup gets equal space for width (1-1-1).
  • All fields (even if not filled) must have their container space. This is correct.
  • Optional: API can introduce new flag which will define proportions between fields, i.e. width={int}. I.e. for two fields we want first to be 75% and second 25% we define per field: width=3 and width=1 (3+1=4 and 3/4=75% and 1/4=25%) like in flex-grow CSS.
@cadavre
Copy link
Contributor Author

cadavre commented Apr 19, 2017

@Dunkat @damianprzygodzki if any aspect of lookup is missing – let me know here.

@cadavre
Copy link
Contributor Author

cadavre commented Apr 25, 2017

Few videos showing how revamp will affect UX:
http://jz-fileshare.s3.amazonaws.com/mf_lookup.zip

@cadavre
Copy link
Contributor Author

cadavre commented Apr 25, 2017

@teosarca @metas-mk We would strongly appreciate if you read and watch all that has been enclosed here and give some feedback about cases (if any) that were not thought here.

@metas-mk metas-mk modified the milestones: 2017-18, 2017-17 Apr 25, 2017
@metas-mk
Copy link
Member

@cadavre thanks. checked. I like very much. workflow and cases fit well and will guide the user much better.

@Dunkat Dunkat self-assigned this Apr 25, 2017
@metas-mk metas-mk modified the milestones: 2017-21, 2017-18 May 16, 2017
Dunkat added a commit that referenced this issue May 17, 2017
Dunkat added a commit that referenced this issue May 17, 2017
Dunkat added a commit that referenced this issue May 17, 2017
Dunkat added a commit that referenced this issue May 17, 2017
Dunkat added a commit that referenced this issue May 17, 2017
@teosarca
Copy link
Member

Hi guys,

Please also consider and fix this case too:
#604 (comment)

@damianprzygodzki
Copy link
Contributor

@teosarca as i told you, this case is not even occurring in new lookup.

@teosarca
Copy link
Member

More, quick talked with @damianprzygodzki and i understood that: when user starts typing into a lookup record, the frontend is PATCHing the value with "NULL" first and then it will PATCH with the new value when user picks one.

I think this approach is not correct because in case we have some validation logics on some fields, it might be that an exception will be thrown when the field is patched with NULL because it's simply not allowed/not a valid value.

More, PATCHing a field with NULL when user did not actually set it to NULL is not natural.

PS: if what i understood is not correct, pls ignore this message.

@cadavre
Copy link
Contributor Author

cadavre commented May 18, 2017

We have this not yet 100% tested and few things are going to change slightly.

Intro:
One thing is that we ALWAYS treat fields in hierarchy parent-child. One field is parent for another one that is positioned to its right (and this one is called child). This assumption is 100% legit for all cases we have – business partner (child: BPlocation, child: ADuser) and product (child: packaging). With this assumption IF we change a parent field – ALL children shall be cleared because their values couldn't be considered as valid. Imagine we change BPartner name and address for BPartner is left from previously selected – sounds weird, right? But if we make such a change we don't want to blink RED to user because he/she is aware that after changing parent – he/she needs one more time to select location and contact person.

tl;dr:
Currently we have bug with two PATCHes, which will be fixed in minutes. As we have assumed on changing parent – we're clearing all children to allow user to select new, proper ones. That is accomplished with sending this (corrected) PATCH:

PATCH
"op": [
   "change": { "parent_field": "new value" },
   "change": { "child1": null },
   "change": { "child2": null }
]

This way backend will have full decision making possibility – to one of: send default values for nulled children OR send back info about validation.

@cadavre
Copy link
Contributor Author

cadavre commented May 18, 2017

About #604 (comment)

We're restoring back OLD value because we NEED to reflect to user what is really saved in backend's database. If user won't make any change – we want to make him aware of what is saved in database – this way we're restoring old value, which makes user perfectly clear about this. Also backend IS NOT aware of that user removed few chars from input and it makes no difference for backend. Imagine case when user goes into input, removes "ed" from already filled and selected "Filled" value and wants to cancel his action (and bring back old value) – how shall he possibly do it? No way. User makes change of selected value ON ACTUAL change, not entering some chars into input, because it's TYPEAHEAD fields which uses entering digits for SEARCHING for possibility TO SELECT.

It's easy to describe and show with real use-case and typeahead flow pattern. :)

@damianprzygodzki
Copy link
Contributor

damianprzygodzki commented May 18, 2017

More, quick talked with @damianprzygodzki and i understood that: when user starts typing into a lookup record, the frontend is PATCHing the value with "NULL" first and then it will PATCH with the new value when user picks one.

Not really, i was proposing that solution to paint it red as you said (to validate field properly in that case). New revamped lookup is reverting changes if there are no changes for model.

Dunkat added a commit that referenced this issue May 24, 2017
Dunkat added a commit that referenced this issue May 26, 2017
Dunkat added a commit that referenced this issue May 26, 2017
Dunkat added a commit that referenced this issue May 29, 2017
damianprzygodzki added a commit that referenced this issue May 29, 2017
Lookup revamp – collective issue #600
@teosarca
Copy link
Member

Quick tested: the Readonly flag is not respected.
Test case:

Dunkat added a commit that referenced this issue May 30, 2017
damianprzygodzki added a commit that referenced this issue May 30, 2017
Readonly flag is not respected #600
@cadavre cadavre modified the milestones: 2017-22, 2017-21 May 30, 2017
@metas-lc
Copy link

metas-lc commented May 31, 2017

found this case while testing another task:

  • go to sales order and create a new one, add a line with batch entry
  • double click on the product and press on x to delete it
    => cursor did not appear (you have to click it to appear, but it should appear directly)

@teosarca
Copy link
Member

found another case:

  • create a new sales order
  • tick the Drop Shipment flag => so the Drop Shipment Partner field becomes visible
  • when the Drop Shipment Partner field is displayed the contact dropdown is automatically opened => it shall not open

image

damianprzygodzki added a commit that referenced this issue Jun 1, 2017
metas-ts added a commit to metasfresh/metasfresh that referenced this issue Jun 1, 2017
[#819](metasfresh/metasfresh-webui-frontend-legacy#819) HTTP header "Accept-Language" shall be correctly set
[#1663](#1663) new order checkup jasper
[#1687](#1687) Refactor Request Tab in Partner Window to allow zoom and advanced edit
[#1239](#1239) Barcode on Bestellkontrolle report
[#428](metasfresh/metasfresh-webui-api-legacy#428) internal: Include AD_Language in layout ETags
[#376](metasfresh/metasfresh-webui-api-legacy#376) Hide Roles on Logon that are not ideal for WebUI
[#426](metasfresh/metasfresh-webui-api-legacy#426) internal: JSONDocument - drop "fields" array field
[#600](metasfresh/metasfresh-webui-frontend-legacy#600) Lookup revamp – collective issue
[#425](metasfresh/metasfresh-webui-api-legacy#425) API support for bookmarking menu items
[#86](metasfresh/metasfresh-webui-api-legacy#86) Check and get rid of "Parameter 'UOMConversion' not found in context" console warnings
[#812](metasfresh/metasfresh-webui-frontend-legacy#812) 1 Key Press save action error during typing
[#1701](#1701) Customer Return Window additional Fields
[#801](metasfresh/metasfresh-webui-frontend-legacy#801) Included row: show row's references in context menu
[#744](metasfresh/metasfresh-webui-frontend-legacy#744) product/packing lookup does nothing when clicking on the single result
[#1640](#1640) fix prepare_services_superuser.sh
[#810](metasfresh/metasfresh-webui-frontend-legacy#810) quickInput endpoints are wrongly called
[#415](metasfresh/metasfresh-webui-api-legacy#415) Notification for Vendor returns jumps to wrong window
[#418](metasfresh/metasfresh-webui-api-legacy#418) Disable zoom into string lookups until it's fixed
[#1644](#1644) Add email validator syntax
[#410](metasfresh/metasfresh-webui-api-legacy#410) Provide view row field zoom-into endpoint

me-45
@metas-mk
Copy link
Member

Tested. Works nicely as required. Closing/ Done.

@metas-mk metas-mk self-assigned this Jun 12, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants