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

Feature request - make ux-dialog-open class configurable #320

Open
ajayvikas opened this issue Jul 28, 2017 · 6 comments
Open

Feature request - make ux-dialog-open class configurable #320

ajayvikas opened this issue Jul 28, 2017 · 6 comments

Comments

@ajayvikas
Copy link

When the dialog is opened the dialog adds ux-dialog-open class to the host and removes it when the dialog is closed. Please make this value configurable through settings.

@StrahilKazlachev
Copy link
Contributor

  1. Please format the issue according to the template or it will be closed.
  2. Present your use case so that we can discuss it.

@ajayvikas
Copy link
Author

my mistake. I thought the template is just for submitting the bug but I looked again I see it is meant for feature request as well. Will format it soon.

@ajayvikas
Copy link
Author

I'm submitting a feature request

  • Library Version:
    1.0.0-rc.1.0.3

Please tell us about your environment:

  • Operating System:
    Windows 10

  • Node Version:
    6.2.0

  • NPM Version:
    3.8.9
  • JSPM OR Webpack AND Version
    JSPM 0.16.32
  • Browser:
    all

  • Language:
    TypeScript 2.4

Current behavior:
When the dialog is opened, the aurelia adds ux-dialog-open class to the host and removes it when the dialog is closed. As I understand this is done to make sure that the use can not scroll the browser when the dialog box is open.

Expected/desired behavior:

  • What is the expected behavior?

  • What is the motivation / use case for changing the behavior?
    There are couple of problem with this approach. If the browser has long content and has scrollbar and we add the ux-dialog-open class to hide the overflow, suddenly the client area increases by the width of the scrollbar resulting the resizing of the all the elements (if the page is responsive). This can be mitigated by adding another style padding-right: [width-of-scrollbar]px;. However the problem is that the scrollbar width is different on different browser. So if we add padding-right:17px, this will work on the firefox and chrome but will not work on edge where the scrollbar width is 12px. Also the scrollbar width on chrome can be customized, so hard coding the scrollbar width is not possible.

One way to mitigate this problem is that the developer can create different classes to handle different browser. e.g.

ux-dialog-open-edge: {
      overflow: hidden;
      padding-right: 12px;
}
ux-dialog-open-chrome: {
      overflow: hidden;
      padding-right: 17px;
} 

and so on.
Also instead of hard coding the ux-dialog-open class, if the developer can specify this class through the dialog setting, they will be able to specify correct class appropriate for their browser or customization (in case of chrome) something like this.

    aurelia.use.plugin("aurelia-dialog", (config: DialogConfiguration) => {
        config.useDefaults();
        var browser = detectBrowserVendor();  //Developer is responsible for detecting the browser version.
       if (browser == "edge")
              config.settings.dialogOpenClass = "ux-dialog-open-edge";
       else if (browser == "chrome" || browser == "firefox")
              config.settings.dialogOpenClass = "ux-dialog-open-chrome";
       other browsers ......
    });

If somebody has better idea, I would love to hear that.

@StrahilKazlachev
Copy link
Contributor

StrahilKazlachev commented Aug 8, 2017

For me allowing for custom marker class is not OK. Changing the marker class will break our and other 3rd party plugins styles.

@ajayvikas
Copy link
Author

I don't understand why it would break anything. You will have a default option, that way the the dialog will behave exactly the same way as it does today.

In any case the problem I described is real. As I said earlier, if anybody have any other solution in mind, I am all ears.

@vermilion
Copy link

At work we need to pass custom class to dialog top level element.
Would be great if this class could also be passed to dialogController.open method.

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

No branches or pull requests

3 participants