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

internet explorer (I know I know..) #13

Open
Dimosthenis opened this issue May 10, 2016 · 18 comments
Open

internet explorer (I know I know..) #13

Dimosthenis opened this issue May 10, 2016 · 18 comments

Comments

@Dimosthenis
Copy link

Thanks for this layout plugin!
Tried to run it on ie10 and edge and failed on both. Haven't checked details yet but on ie10 the console throws an error and on edge it just won't work. Will come back with more info if possible.

Any ideas?

@shchukin
Copy link

Same here.

Can be considered to use fallback like so:

.bricklayer > * { width: 150px; }

This selector will work only when plugin is disabled or not working.

@shchukin
Copy link

Maybe should update a readme?
This plugin works seamlessly with, Safari, Firefox, Chrome and all other modern browsers.

@f
Copy link
Collaborator

f commented May 16, 2016

We didn't tested on IE but I did not expect that it would fail on IE Edge.

We'll test it first and we'll update it I think.

_Fatih Kadir Akın_http://github.com/f
http://twitter.com/fkadev

@Loac-fr
Copy link

Loac-fr commented May 17, 2016

If it can help, both IE10 and 11 throw an error here (line 132) :

Container.prototype.getElementsInOrder = function () {
    return this.element.querySelectorAll(":scope > *:not(." + this.options.columnClassName + "):not(." + this.options.rulerClassName + ")");
};

It's probably the same for Edge.

Error is :
SCRIPT5022 syntax error - line 132 character 13

They are supposed to handle querySelectorAll so it might be the expression ?

Edit
More specifically the issue seems to be linked with the use of :scope
See https://developer.mozilla.org/en-US/docs/Web/CSS/:scope#Browser_compatibility

@evoactivity
Copy link

Is :scope even needed there?

@f
Copy link
Collaborator

f commented May 24, 2016

@evoactivity Unfortunately, yes. We need it. You have to use :scope to use > selector.

If you find a better way to make it let's do it.

@Dimosthenis
Copy link
Author

Needs a lot of testing but managed to make it work on ie 10.

In bricklayer.js line 14
" if (window["CustomEvent"]) {.. "
changed it to
" if (window["CustomEvent"] instanceof Event) { "

and on line 123 and line 132
changed " :scope " to " .bricklayer "

Its not the best solution if someone wants to change classes but its something

@RobSchoenaker
Copy link

RobSchoenaker commented Jun 8, 2016

This appears to work for all browsers:

            Container.prototype.getColumns = function () {
                return this.element.querySelectorAll("." + this.options.columnClassName);

            };

            Container.prototype.getElementsInOrder = function () {
                return $(this.element).children().select("*:not(." + this.options.columnClassName + "):not(." + this.options.rulerClassName + ")");
            };

Keep in mind that the :scope selector is a candidate or deletion. It's an experimental selector.

@f
Copy link
Collaborator

f commented Jun 8, 2016

May you open a PR?

8 Haz 2016 Çar 14:47 tarihinde RobSchoenaker [email protected]
şunu yazdı:

This appears to work for all browsers:

        Container.prototype.getColumns = function () {
            return this.element.querySelectorAll("." + this.options.columnClassName);

        };

        Container.prototype.getElementsInOrder = function () {
            return $(this.element).children().select("*:not(." + this.options.columnClassName + "):not(." + this.options.rulerClassName + ")");
        };


You are receiving this because you commented.

Reply to this email directly, view it on GitHub
#13 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AAL_fbdnIs11V2hZBWCOpjr4_pwBOZvfks5qJqvqgaJpZM4Ibb9v
.

_Fatih Kadir Akın_http://github.com/f
http://twitter.com/fkadev

@RobSchoenaker
Copy link

I am not so good with GitHub, but I tried to create a PR. Did not work for now.. Also Typescript is not my language, but I suppose your fix is like below. Feel free to take credit.

in bircklayer.ts

private getColumns() {
  return this.element.querySelectorAll(`.${this.options.columnClassName}`)
}

private getElementsInOrder() {
  return $(this.element).children().select(`*:not(.${this.options.columnClassName}):not(.${this.options.rulerClassName})`)
}

@evoactivity
Copy link

I can confirm that the fix @RobSchoenaker suggested doesn't actually work.

I'm also having issues with bricklayer and any version of IE, not just old versions, it's one things to say "other modern browsers" to suggest older IE's wont work, but literally no version of IE works with bricklayer. That really needs to be made explicitly clear in the readme. I'm going to see what I can do as it seems related to the :scope selector but I don't understand why bricklayer needs this at all.

@evoactivity
Copy link

Ok, so using this https://github.com/lazd/scopedQuerySelectorShim and always using the custom event fallback works in ie10+

@RobSchoenaker
Copy link

@evoactivity The fix I suggested works in my (maybe specific) case. Can you PR your update to @f so we can all benefit from this?

@f
Copy link
Collaborator

f commented Jul 6, 2016

As I understand using the shim externally will solve the problem. Means no need to change codebase. Right @evoactivity?

@evoactivity
Copy link

@RobSchoenaker When I applied your fix it caused issues fetching the correct blocks and broke the layout, will have another look at that when I have some time in the next few days.

@f pretty much yeah, the shim solves the problem for the most part, but the custom event fallback wasn't working in IE, I simply commented out the check to look for the existence of CustomEvent and always use the fallback. I'm not sure why the check didn't work, I was on a tight deadline so didn't have the time to properly investigate, again will look into it when I have time.

@kilianso
Copy link

I can confirm that @evoactivitys solution with scopedQuerySelectorShim + get rid of the conditional and use only the CustomEvent solves the problem.

I tested it with:
IE 10,11
Edge 13,14
Chrome >50
Firefox >45
Safari 8,9,9.1
Chrome 51 on Android 4.4, 5,6
Safari 9,8,7 on iOS 8,9

@f You should include this solution directly into bricklayer.js, because it's an enhancement probably every dev will search for. It's not even working in latest Edge right now which is a modern browser (says Microsoft ;) ) Or maybe find a similar solution without the need of adding the SelectorShim.

But i love it anyway, it's dependency-free, lightweight and superfast! Great work! Will create another issue for a slow-connection/no-js fallback enhancement.

@kilianso kilianso mentioned this issue Dec 7, 2016
3 tasks
@benjamminf
Copy link

The quickest way to get this working is to include polyfills for :scope and CustomEvent found here:

Load both of these before Bricklayer and you'll be working in IE10/11 and Edge.

@MattAppleton
Copy link

MattAppleton commented Apr 28, 2018

@benjamminf — thanks for the tip, that does seem to help, now I just need to address text styling which has gone funny on my bricklayer items — how I hate IE 11!!

https://gresfordarchitects.co.uk/projects

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

9 participants