Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds cursor: pointer; to all inputs #105

Open
adambedford opened this issue Jan 8, 2018 · 48 comments
Open

Adds cursor: pointer; to all inputs #105

adambedford opened this issue Jan 8, 2018 · 48 comments
Assignees

Comments

@adambedford
Copy link

I'm seeing an inline style cursor: pointer; added to input elements (along with touch-action: manipulation; -ms-touch-action: manipulation;), which isn't desirable as it breaks convention for fields that accept user input.

I'm pretty sure the source of this inline style is ember-toggle and this seems like a bug.

@knownasilya
Copy link
Owner

What is the style that's introducing this in your app?

We don't use any generic selectors, everything is behind a specific toggle class. The css is here https://github.com/knownasilya/ember-toggle/tree/master/vendor/ember-toggle

@lolmaus
Copy link
Collaborator

lolmaus commented Jan 8, 2018

@knownasilya, he's talking about inline styles, the ones set from JS.

@knownasilya
Copy link
Owner

I don't think we have any in v5.

@lolmaus
Copy link
Collaborator

lolmaus commented Jan 8, 2018

@adambedford, can you repro?

@adambedford
Copy link
Author

adambedford commented Jan 8, 2018

here's an example pulled from the Chrome inspector. It happens with input elements as well. I'm not 100% positive this is coming from ember-toggle but I can't think of where else it'd be coming from at the moment

<textarea 
  style="touch-action: manipulation; -ms-touch-action: manipulation; cursor: pointer;" 
  placeholder="Add a private note..." 
  id="ember1260-field"
  class="form-control ember-view">
</textarea>

@lolmaus
Copy link
Collaborator

lolmaus commented Jan 8, 2018

This addon has nothing to do with textareas. Try running your browser with all extensions disabled (there should be a CLI flag for that).

@knownasilya
Copy link
Owner

Maybe ember-gestures does something? This addon does use that addon..

@danconnell
Copy link

This is happening because ember-hammertime, a dependency of this addon, is applying those styles

@lolmaus
Copy link
Collaborator

lolmaus commented Jan 12, 2018

@danconnell Thank you!

Closing this in favor of upstream issue: html-next/ember-hammertime#31

@lolmaus lolmaus closed this as completed Jan 12, 2018
@danconnell
Copy link

FYI @adambedford ember-hammertime doesn't have working configs, as documented in that bug, so you're best off downgrading to ember-toggle@5.0.2 until ember-hammertime is fixed.

@knownasilya
Copy link
Owner

@rwwagner90 just including you since you are active in the ember-hammertime repo.

@adambedford
Copy link
Author

thanks @danconnell Unfortunately I need the material style so I can't downgrade. I'll have to wait this one out I suppose.

@RobbieTheWagner
Copy link
Collaborator

@knownasilya yeah, the issue in ember-hammertime was being worked on by @Techn1x but he said he was not having much success. What is the desired fix here? If we make it more configurable, can ember-toggle disable some of these? Just want to know exactly what we want before I look into a fix.

@knownasilya
Copy link
Owner

Basically, it makes all inputs have a pointer on hover. Not sure we should be doing anything from our side. I think the desired effect is that only items that need touch support via hammertime have the pointer. Maybe the API there should be opt-in for elements. I'd be fine with having to explicitly opt ember-toggle into touch.

@RobbieTheWagner
Copy link
Collaborator

@knownasilya do we need ember-gestures for this addon?

@knownasilya
Copy link
Owner

knownasilya commented Jan 14, 2018

It might be overkill, we just need swipe/drag support (so toggles feel native), maybe there is a simpler way to do that.

@lolmaus
Copy link
Collaborator

lolmaus commented Jan 14, 2018

Using ember-gestures was the simplest thing implementation-wise. Native drag events are pretty quirky.

@knownasilya
Copy link
Owner

Yeah, which is sad :(

@lolmaus
Copy link
Collaborator

lolmaus commented Jan 16, 2018

We're using this as a workaround until html-next/ember-hammertime#31 is resolved and included into ember-gestures:

input[type="text"], textarea {
  cursor: text !important;
}

@RobbieTheWagner
Copy link
Collaborator

I just released ember-hammertime 1.3.0, with configuration options that should allow us to disable these things. Is anyone interested in helping get this into ember-gestures and over the finish line, so we can fix this here?

@adambedford
Copy link
Author

@rwwagner90 How do we get your fix into ember-toggle to resolve this issue?

@RobbieTheWagner
Copy link
Collaborator

@adambedford I don't think anything needs to be done here. I think ember-hammertime is installed directly in your app via a blueprint, so you should just have to configure it.

@adambedford
Copy link
Author

Thank you @rwwagner90 Do you have guidance on what the ember-hammertime config should be to get rid of the inline styles for all other elements?

@RobbieTheWagner
Copy link
Collaborator

@adambedford the configuration is documented in the README https://github.com/html-next/ember-hammertime#configuration

If anything is unclear, please let me know.

@knownasilya
Copy link
Owner

knownasilya commented Feb 16, 2018

So something like touchActionSelectors: ['.x-toggle-component']?
We can probably document a good default for x-toggle.

@RobbieTheWagner
Copy link
Collaborator

RobbieTheWagner commented Feb 16, 2018

@knownasilya that might work. All of the config options need to be set though. The default is to apply the styles to everything with an action all buttons, etc. and if we want to turn all that off, it needs to be something like:

EmberHammertime: {
    touchActionOnAction: false,
    touchActionAttributes: [''],
    touchActionSelectors: ['.x-toggle-component'],
    touchActionProperties: 'touch-action: manipulation; -ms-touch-action: manipulation; cursor: pointer;'
  }

We don't want to mess up ember-hammertime for the consuming app by assuming any config, but we should document how to disable it.

@adambedford
Copy link
Author

I haven't been able to get this working. I tried the config suggested by @rwwagner90 and even installed ember-hammertime in my app directly (it doesn't seem to be installed by ember-toggle via blueprint, but is instead a dependency)

@RobbieTheWagner
Copy link
Collaborator

@adambedford I thought we didn't have a direct dep on it here, but it appears I was wrong. We need to update the deps here to use ember-hammertime 1.3.0.

@adambedford
Copy link
Author

@knownasilya @rwwagner90 Has there been any progress on this by chance?

@RobbieTheWagner
Copy link
Collaborator

@adambedford I just opened a PR to update ember-hammertime here.

@adambedford
Copy link
Author

I've updated to the master branch of this library and have the following configuration in environment.js but I'm still seeing the unwanted styles set on almost every element in my application

EmberHammertime: {
      touchActionOnAction: false,
      touchActionAttributes: [''],
      touchActionSelectors: ['.x-toggle-component'],
      touchActionProperties: 'touch-action: manipulation; -ms-touch-action: manipulation; cursor: pointer;'
    }

Any pointers on what I need to do to solve this?

@knownasilya
Copy link
Owner

We might have to set this config in ember-toggle.. I'll give it a try tomorrow.

@knownasilya knownasilya reopened this Mar 18, 2018
@RobbieTheWagner
Copy link
Collaborator

@adambedford @knownasilya are we using the touch action mixin anywhere or just allowing the default behavior?

@knownasilya
Copy link
Owner

@adambedford give master another try. I merged another version bump change.

@knownasilya
Copy link
Owner

If that doesn't work, there might be some issue that doesn't propogate the config to the hammertime addon. We might make it an install "add npm package" feature instead of bundling together.

@RobbieTheWagner
Copy link
Collaborator

@knownasilya I don't think the config is passed down because it's a nested dep. Definitely would like to explore how we might fix this.

@adambedford
Copy link
Author

Could the configuration simply be added to ember-toggle?

@RobbieTheWagner
Copy link
Collaborator

@adambedford I don't think we want that. We want users to be able to change the ember-hammertime config directly.

@adambedford
Copy link
Author

Fair enough. Any idea how we fix this?

@RobbieTheWagner
Copy link
Collaborator

@adambedford I would need to play with it. The config is definitely overridable in ember-hammertime, itself, so we would need to investigate if we can pass it down from this addon or if we need to have ember-hammertime not be a dep and get installed via a blueprint. Would you be interested in working on a PR to explore these options?

@adambedford
Copy link
Author

I wasn't able to dedicate any time to this unfortunately. Has anyone had a chance to make progress?

@adambedford
Copy link
Author

Here's a quick fix derived from @lolmaus above:

[style^="touch-action: manipulation"]:not(.x-toggle-btn, button, a) {
  cursor: inherit !important;
}

@knownasilya
Copy link
Owner

I'll have a look soon. Thanks for bringing it up again.

@knownasilya knownasilya self-assigned this Dec 19, 2018
@RobbieTheWagner
Copy link
Collaborator

I think we probably just don't need hammertime anymore, these days. What do you think @knownasilya?

@knownasilya
Copy link
Owner

@rwwagner90 yeah, seems a bit overkill. Would have to add slide support manually though.

@RobbieTheWagner
Copy link
Collaborator

@knownasilya is it not possible to use ember-gestures without ember-hammertime?

@knownasilya
Copy link
Owner

Oh, totally not sure. This is the only experience I have with either.

@knownasilya
Copy link
Owner

@adambedford give master a try. I've removed hammertime and it seems to have done the trick.
cc @rwwagner90

If it works for you, then I'll publish a release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants