popup: split own functionality into target modifiers

[veged]

Update: See comments below for discussion. Final case: #968 (comment).

---

First of all -- we need good name for it! ;-)

It will provide functionality about:

1. close on Esc key
2. correct closing within "virtual nodes tree"

[narqo]

After having some small meeting \w @veged @dfilatov we have an idea to split current popup into several consistent blocks (names would be discussed):

• base-popup – implements common logic about virtual nested nodes, opening/closing, etc;
• popup with target – popup which could open in respect to some "anchor element" (target);
• popup with coords – popup which could open in exact coordinates;
• modal popup – a special case of popup which always opens in the center of the screen.

The downside of this decision is that we would have several more blocks instead of one block to rule them all.

[narqo]

Another possible approach is to have a single popup block and implement all the different behaviours as block modifiers, e.g. popup_type_popover, popup_type_modal.

Personally I don't like this idea, "modal popup" and "popup with target" would have different behaviour, as well as different implementation and CSS styles (according to our design guides). IMHO: it doesn't fill like a good architectural decision – different classes should be implemented in different consistent modules.

[narqo]

/cc @dfilatov @Coltspb let's eat 🐝 :)

[narqo]

The thing I've noticed recently: this kind of abstraction could (well, maybe, would..?) be useful to implement suggest that we have on the search page of yandex.ru:

[persidskiy]

After having some small discussion with myself, I think it may be unnecessary having this "abstract block". Because I haven't found any cases where this block actually needed(except popup and modal blocks). I've estimated this cases:

• suggest - is regular popup + it can't contain any popups.
• advanced search at SERP - closes selects well because of _autoclosable on popups
• filters at yandex.auto - closes selects well by the same resason

Another approach (as @narqo mentioned) is spliting popup into two modifiers:

popup               - a little common js for modal and popover
    _type
         _popover   - much js about calculating positions and subscriptions to parent popups and owner
         _modal     - a little js about modal
    _theme
         _normal-popover
         _normal-modal
         _simple
    _autoclosable

Pros:

• Since popover recognises parent modal as popup, all our "virtual dom" and bindings to parent popup works well
• common _autoclosable
• 1 block instead 3 or 4`

Cons:

• mandatory modifier _type
• different API (popover has .redraw(), .setTarget() + some parametres in addition) and tests
• different _theme for modal and popover

I've a prototype with it, would you like to look at in WIP PR? :)

[veged]

@Coltspb popup_theme_normal-{popover,modal} looks ugly — it mix theme with behaviour

advanced search at SERP has selects inside himself (as well as suggest may have some "popups", for example tooltips)

[veged]

@Coltspb also I'm pretty unsure about popup and popover words — does it clear to understand the difference for non-native speakers (also for native too ;-))??

[persidskiy]

@veged the separation of theme into two modifiers has been made only for assets loading optimization. We can combine those themes in popup_theme_normal like we did in button with _view modifier.

Yep, advanced search has selects, but they closes well by clicking on body. And suggest is regular popup, I don't see any problem with popovers inside himself (if popovers implemented using popup).

Naming is the common problem ;)

[veged]

@Coltspb let's discuss it on meeting today at 16:00

[dfilatov]

After new conversation we considered following variant as the best one:

• popup – all basic stuff about "virtual nesting"
  • _target – common between popups with target as an anchor node and target as a position
    • _anchor – bound to some dom-node
    • _position – shown at some position
  • _autoclosable – stuff about outside clicks and Esc key
• modal – mixed with popup_autoclosable
• tooltip – mixed with popup_target_anchor and optional with popup_autoclosable

[persidskiy]

Wowowow, sounds good. As far as I understand, we can still use popup without _target and _theme for some inline block which should close inner popups by escape or outside click, can we?

[narqo]

@Coltspb What do you mean by "[..] for some inline block [..]"? Could you illustrate this case with some example?

[persidskiy]

@narqo We discussed this at our first meeting, when we decided to create abstract block which should be mixed to popup or modal as well as to "some inline block which should close inner popups". For example, closable comment tree at my.at.yandex.ru or advanced search at SERP.