Added `on` argument with `hover` and `click` value. by ivansglazunov · Pull Request #12 · lookback/meteor-dropdowns

ivansglazunov · 2016-03-14T05:05:59Z

Use this version of your code for some time.
Thank you for your cool package!

ivansglazunov · 2016-03-14T05:26:46Z

What is the name for method usePosition?

ivansglazunov · 2016-03-14T05:27:35Z

Make it accessible from the outside makes it possible to write an easy change of logic for the trigger.

johanbrook · 2016-03-14T18:23:39Z

lib/dropdown.js


 Template.dropdownTrigger.events({
  click(evt, tmpl) {
+    if (tmpl.data.on && tmpl.data.on != 'click') return;


Please use !== if possible.

johanbrook · 2016-03-14T18:30:35Z

Tracker.afterFlush(positionDropdown(name, tmpl.find(DROPDOWN_TRIGGER)));

While I see the need for using the tmpl instance in here, I think we shouldn't mix the templates with business logic in the Dropdowns namespace. I think users should pass either a DOM node or a string selector in the second argument to this method.

ivansglazunov · 2016-03-14T22:29:46Z

I will do all 3 options: selector, element, template.
The template will look for template.$( '>').First()[0].
Ok?

johanbrook · 2016-03-15T12:26:46Z

Sure, go for it :)

ivansglazunov · 2016-03-16T05:26:48Z

Done.
Documentation updated too. Maybe need an adjustment :)

ivansglazunov · 2016-03-16T05:31:21Z

Dropdowns.get not returns real option on and timeout. And API section in test-app need to update. I'll fix it now.

ivansglazunov · 2016-03-16T05:38:22Z

Hmm...2 variants of solutions:

Move on andtimeout in dropdown from dropdownTrigger
Accept that they are not properties of dropdown

johanbrook · 2016-03-16T12:36:44Z

README.md

 - `direction` - One of `n`, `s`, `e` or `w`. Where to position the dropdown around the element. Defaults to `s`.
 - `persistent` - Defaults to `false`. Set to `true` if you want the dropdown *not* to hide when clicking outside it (on `document`).
 - `on` - Defaults to `click`. Set to `hover` for respond to the pointing of the mouse. Set `none` for disable trigger logic. You can use `usePosition` for write your own reactions logic.
+- `timeout` - Defaults is `0`. Set a number to have time to move the mouse from the `dropdownTrigger` on the `dropdown`.


Is an option for timeout really necessary to include in this package?

What have you decided with options?

Move to the dropdown, or remove from its default object and leave only in dropdownTrigger?

Can't timeout be a hard coded constant somewhere, instead of an option?

ivansglazunov · 2016-03-16T12:39:31Z

This allows you to leave the dropdown available when hover mode.
If need to change the names of fields / options, just tell.
Now fix spacing.

ivansglazunov · 2016-03-16T12:40:26Z

Without a timeout, hover become only-tooltip implementation. Without the ability to use a dropdown active zone.

johanbrook · 2016-03-16T12:43:17Z

lib/dropdown.js

  };
 };

+Dropdowns._timeout;


Actually no need to define this here.

Rewrite it to?

var timeout;

No need to declare it – you'll attach it further down, at this line anyway, and JS/Meteor doesn't care if the parameter passed to Meteor.clearTimeout is undefined.

I try. It does not work.

johanbrook · 2016-03-16T12:49:09Z

If you wanna check the code style, do

npm install
npm run lint

:)

johanbrook · 2016-03-16T12:57:10Z

You're also welcomed to add unit tests for the various stuff on the Dropdowns object.

ivansglazunov · 2016-03-16T12:57:45Z

Tests. Ok.

ivansglazunov · 2016-03-16T13:14:02Z

Why mocha, not tinytest?

johanbrook · 2016-03-16T13:14:54Z

I like its API more.

…ger argument.

ivansglazunov · 2016-03-16T13:37:06Z

How to styled separate dropdown arguments from dropdownTrigger arguments... hmmm

ivansglazunov · 2016-03-16T13:38:01Z

dirty br now...

ivansglazunov · 2016-03-16T13:38:56Z

May be create 2 headers?

Additional dropdown arguments
Additional dropdownTrigger arguments

johanbrook · 2016-03-16T13:39:29Z

May be create 2 headers?

👍 Please do.

ivansglazunov · 2016-03-16T13:41:17Z

It's too long. Be shortened for the sake of design. How now?

johanbrook · 2016-03-16T13:43:17Z

Just try "Dropdown arguments" and "Trigger arguments". And remember to update the table of contents at the top too.

ivansglazunov · 2016-03-16T13:47:41Z

May be done.

ivansglazunov · 2016-03-16T13:49:56Z

I wonder whether it is possible then to use this package with the svg on d3 ... If I find a way to use usePosition then necessarily write about it here.

johanbrook · 2016-03-16T13:52:53Z

Ready for merge?

ivansglazunov · 2016-03-16T13:54:01Z

It is also necessary to specify the version. Better recheck me.)

johanbrook · 2016-03-16T13:54:44Z

It's okay, I'll sort publishing details later :)

ivansglazunov · 2016-03-16T13:55:36Z

I want to check the possibility of recursive dropdown.

ivansglazunov · 2016-03-16T13:58:26Z

Look. This is if disable 393 and 403 lines.

ivansglazunov · 2016-03-16T13:59:04Z

Maybe something come up that were possible? It's cool!

johanbrook · 2016-03-16T13:59:30Z

What do you mean with "recursive dropdown"? Let's not do that in this PR either way.

ivansglazunov · 2016-03-16T13:59:39Z

Yes

ivansglazunov · 2016-03-16T13:59:54Z

Go to issues or talk here?

ivansglazunov · 2016-03-16T14:00:10Z

Check the test page.

ivansglazunov · 2016-03-16T14:00:21Z

There's an example.

johanbrook · 2016-03-16T14:01:56Z

Cool.

I don't know if this is in scope of this package though, I don't think recursive dropdowns is a nice UI concept.

ivansglazunov · 2016-03-16T14:02:15Z

Need to add ignore field.

ivansglazunov · 2016-03-16T14:04:31Z

  {{#dropdownTrigger on="hover" timeout=500 name="recursiveTest1"}}
    <button id="dropdown1">Trigger #1</button>
  {{/dropdownTrigger}}

  {{#dropdown name="recursiveTest1" top="10"}}
    {{#dropdownTrigger on="hover" timeout=500 name="recursiveTest2"}}
      <button id="dropdown1">Trigger #2</button>
    {{/dropdownTrigger}}

    {{#dropdown name="recursiveTest2" in="recursiveTest1" top="10"}}
      {{#dropdownTrigger on="hover" timeout=500 name="recursiveTest3"}}
        <button id="dropdown1">Trigger #3</button>
      {{/dropdownTrigger}}

      {{#dropdown name="recursiveTest3" in="recursiveTest3" top="10"}}
        <p>Hello world</p>
      {{/dropdown}}
    {{/dropdown}}
  {{/dropdown}}

Then in click and mouseover
hideAllBut(tmpl.data.name, ...ins);

ivansglazunov · 2016-03-16T14:05:36Z

To get all the previous need to recursively ask parents dropdown instances about the field in...

ivansglazunov · 2016-03-16T14:06:22Z

I can do it now, or after merge. How will be better?

johanbrook · 2016-03-16T14:07:55Z

Again, I'm not sure I wanna include recursive dropdowns at all in this package – if you need it, please fork and use that in your projects.

ivansglazunov · 2016-03-16T14:11:52Z

Ohh... I was inattentive.
How can I influence this decision? It may be worth taking this package not only for dropdowns but also for the dropmenu / tooltips? Then this functionality would be universal. I do not mind to fork, but cleanly for themselves, completely not interesting)

ivansglazunov · 2016-03-16T14:21:45Z

I don't think recursive dropdowns is a nice UI concept.

I agree almost no good examples of this. However, does this mean that it is not necessary to implement an ability? When creating editor-application with many options it can be quite handy. Or I have no chance?)

ivansglazunov · 2016-04-06T18:59:18Z

http://meteor-templ.herokuapp.com/drop

ivansglazunov added 2 commits March 14, 2016 08:04

Added argument with hover value.

642a03e

Method usePosition.

e103c84

ivansglazunov added 2 commits March 14, 2016 08:29

README.md

ea80797

Remove consolc.log...

d64ccd4

johanbrook reviewed Mar 14, 2016
View reviewed changes

usePosition selector fix and timeout option

732ee2c

johanbrook reviewed Mar 16, 2016
View reviewed changes

Fix spacing and {}.

25d46d5

johanbrook reviewed Mar 16, 2016
View reviewed changes

ES2015

bd01061

Fix tabs.

8a9c30a

Remove timeout and on from default and atts. Now it only dropdownTrig…

532c08f

…ger argument.

fix

252f7b1

Docs fixs.

2212939

Conversation

ivansglazunov commented Mar 14, 2016

Uh oh!

ivansglazunov commented Mar 14, 2016

Uh oh!

ivansglazunov commented Mar 14, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

johanbrook commented Mar 14, 2016

Uh oh!

ivansglazunov commented Mar 14, 2016

Uh oh!

johanbrook commented Mar 15, 2016

Uh oh!

ivansglazunov commented Mar 16, 2016

Uh oh!

ivansglazunov commented Mar 16, 2016

Uh oh!

ivansglazunov commented Mar 16, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ivansglazunov commented Mar 16, 2016

Uh oh!

ivansglazunov commented Mar 16, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

johanbrook commented Mar 16, 2016

Uh oh!

johanbrook commented Mar 16, 2016

Uh oh!

ivansglazunov commented Mar 16, 2016

Uh oh!

ivansglazunov commented Mar 16, 2016

Uh oh!

johanbrook commented Mar 16, 2016

Uh oh!

ivansglazunov commented Mar 16, 2016

Uh oh!

ivansglazunov commented Mar 16, 2016

Uh oh!

ivansglazunov commented Mar 16, 2016

Uh oh!

johanbrook commented Mar 16, 2016

Uh oh!

ivansglazunov commented Mar 16, 2016

Uh oh!

johanbrook commented Mar 16, 2016

Uh oh!

ivansglazunov commented Mar 16, 2016

Uh oh!

ivansglazunov commented Mar 16, 2016

Uh oh!

johanbrook commented Mar 16, 2016

Uh oh!

ivansglazunov commented Mar 16, 2016

Uh oh!

johanbrook commented Mar 16, 2016

Uh oh!

ivansglazunov commented Mar 16, 2016

Uh oh!

ivansglazunov commented Mar 16, 2016

Uh oh!

ivansglazunov commented Mar 16, 2016

Uh oh!

johanbrook commented Mar 16, 2016