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

Add option to set parent element as <input> element container #204

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

behz
Copy link

@behz behz commented Nov 8, 2017

Hello,

First off a big thanks to anybody contributed to this great plugin.

WHAT PROBLEM DOES THIS PR SOLVE?
There would be some situation where the upload button could be really large like what we have in our app https://cl.ly/0M3Y1e313Z3v .  When user scrolls, the transparent element covers the whole sticky nav bar, so a click on nav bar buttons, opens the uploader instead.
Additionally I can see some situations where upload button is positioned fixed or is on a sticky parent, then scrolling the page would move the element which is positioned relative to document element while button is still in the sticky position.

SOLUTION:
Adding two new options: relativeToParent and inputZIndex
Because the element is added to the body(with a huge z-index), there is no workaround to fix problems mentioned above except having the element added on the parent element of the button(s).
This way the position and z-index of the upload element is bound to the parent of button, or closest ancestor of all upload buttons on the screen.

CHANGES:
Expose 2 new config params through the API to set container for file input elements (document by default) and Z-Index:

  1. relativeToParent: OPTIONAL Boolean
    False (default):  Use document element as the container for the file element
    True: Use parent element or a common ancestor for all upload buttons as the container for the file element
  2. inputZIndex: OPTIONAL Number -
    Explicitly set Z-Index of file input elements added to DOM. Default: 16777270

NOTE:
Docs have NOT been updated, but I didn't want to do that until I knew whether or not this PR might be accepted.

I have tested the code in many situations and we are currently using it in a production webapp. Everything is working as expected without any problem.

I really appreciate if you merge it back in master branch.

Thank you!
Behzad.

…ut> element will be added on the parent element of the button or common parent of multiple buttons.
…ativeToParent` is true. to prevent covering over other elements on the page e.g sticky header.
@SterlingVix
Copy link

@LPology This change to set the parent / Z-index is a requested feature in your issues backlog:

#145

Can you please merge this is or provide some feedback?

@SterlingVix
Copy link

@LPology @sdturner02 -- Is there any chance of this being merged in? Can you provide some feedback please?

We've been maintaining a custom branch of this repo just to support this feature. We'd really like it merged in.

Can I help in any way?

@LPology
Copy link
Owner

LPology commented Jan 5, 2019

First, let me apologize for taking so long to reply. Without going into details, I was unable to work on this project for a number of months.

I like this, but it appears to break the plugin in IE7, IE8, and IE9. I would like to be able to continue to support those browsers if possible - I know of several users of the plugin who need to support old versions of IE. I do as well.

In IE9, the file select button will work on initial page load, but stops working when the page is scrolled. When scrolling back to the original position, it starts working again. In IE8 the button does not work at all.

Any thoughts on how we can make it work in old versions of IE?

@SterlingVix
Copy link

@LPology - thanks for the update. Hopefully whatever has prevented you from maintaining this is over, and your life is improved. :)

@behz - let's work together to ensure this works on IE7-9.

@behz
Copy link
Author

behz commented Jan 15, 2019

@LPology Thanks for getting back to this.

Unfortunately, I don't have IE to test now. I just reviewed my changes. The only syntax that seems is not supported in IE7-9 is getComputedStyle(). I just pushed a fix to use ss.getStyle function instead getComputedStyle to fix the bug.

@LPology Can you please try again and let me know if it fixes the bug?

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

Successfully merging this pull request may close these issues.

3 participants