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

Browsers without CSS variables are broken #630

Closed
RussellAbraham opened this issue Nov 2, 2020 · 19 comments
Closed

Browsers without CSS variables are broken #630

RussellAbraham opened this issue Nov 2, 2020 · 19 comments
Labels
Bug resolved if issue is resolved, it will be open until merge with master

Comments

@RussellAbraham
Copy link

I have question related to jQuery Terminal

Hi, first time issuing a question.

  • CSS variables
  • Map function

Is causing compatibility issue with some of the mobile platforms you have detection for.

Do you plan to include a Map polyfill and alternative to CSS variables?

Thanks.

@jcubic
Copy link
Owner

jcubic commented Nov 3, 2020

I've planed to add to documentation how to use css variables ponyfill (I've played with it on IE11) I need to test if it's still usable without this, but I don't think it's good idea to include with the library. As for Map I should probably add polyfill. That will be included.

@jcubic
Copy link
Owner

jcubic commented Nov 3, 2020

Can you tell me what platforms are you interested that don't have Map and CSS variables?

@RussellAbraham
Copy link
Author

Can you tell me what platforms are you interested that don't have Map and CSS variables?

Only one actually. BlackBerry 10, Webkit 10.3.3.3216.

After adding a polyfill for Map.

IMG_20201103_163256

@jcubic
Copy link
Owner

jcubic commented Nov 4, 2020

Not sure if I can easily test this, on I don't think that there is such platform at BrowserStack. But I can make it work better with CSS variables or maybe add workaround that can be used.

Did you change the colors anyway or is this the default? At least it should be all black without CSS variables you only should have problems with setting for instance white backround with css variables.

Will test on IE11 and see what can I do. If you will be able to test on your blackberry that would be really helpful.

@RussellAbraham
Copy link
Author

No that is the default theme from your distribution.

Sure, I tried to produce a heap yesterday but the devices debugger just output a blank file and I had some errors from Backbone that just added a layer of confusion.

@jcubic
Copy link
Owner

jcubic commented Nov 16, 2020

Also one more question do you have any global css on the page? Maybe * selector that is applied to the page or maybe selectors like span or div?

@RussellAbraham
Copy link
Author

Also one more question do you have any global css on the page? Maybe * selector that is applied to the page or maybe selectors like span or div?

No not at all, just what is installed from v2.19.2. I have been using v0.10.1 until I read through the whole library.

@jcubic
Copy link
Owner

jcubic commented Nov 18, 2020

Are you sure you're using same version of CSS and JS? I've checked demo on website in IE11 and it works fine without css variables.

@RussellAbraham
Copy link
Author

RussellAbraham commented Nov 19, 2020

Yea, adding styles my self would be no problem. I deployed demos of the latest and v0.10.11 here.

Github Pages

I'll add some screen shots of before and after. At least up to this style sheet issue. No stress, just some stats.

I was hoping to use this browser specifically, android Fennec browser paints everything fine.

Thanks.

@jcubic
Copy link
Owner

jcubic commented Nov 19, 2020

Thanks for the demos. Running in Browserstack in IE11 shows the error. I think that demo (that I've tested) have background from the page not from terminal. Will investigate and try to fix the issue.

It seems that I completely forgot to test in IE11. In previous version I've tried to make it work in that browser. And as this issue suggest there are more browsers that don't support css variables.

@jcubic jcubic added the Bug label Nov 19, 2020
@jcubic
Copy link
Owner

jcubic commented Nov 19, 2020

Quick fix before I update the library is to just use this for now:

.terminal {
   background: black;
}

@jcubic jcubic changed the title Compatibility Browsers without CSS variables are broken Nov 19, 2020
jcubic added a commit that referenced this issue Nov 19, 2020
* Fix background color
* Fix cursor animations with prefix class
@jcubic
Copy link
Owner

jcubic commented Nov 19, 2020

CSS part is fixed, I only need to also fix #631 for version 2.19.3.

I also have questions about Map I've tested in Safari 10 on MacOSX Sierra and there are no issues. Most of the features of Map are in Safari 8, it even work in IE11. Map is used in code that cache the processing of long command line in multiline command when you just move the cursor. It seems that there are no issues. Do you have any exceptions from Map being undefined?

@RussellAbraham
Copy link
Author

RussellAbraham commented Nov 20, 2020

CSS part is fixed, I only need to also fix #631 for version 2.19.3.

I also have questions about Map I've tested in Safari 10 on MacOSX Sierra and there are no issues. Most of the features of Map are in Safari 8, it even work in IE11. Map is used in code that cache the processing of long command line in multiline command when you just move the cursor. It seems that there are no issues. Do you have any exceptions from Map being undefined?

No, it doesn't really need Map. With Map not being built in, an error was thrown from :L312.

I am pretty sure I included the polyfill only to avoid the error in the console. After Map was fixed an error for css variables was thrown and I know this version of safari, sadly, does not have variables built in. I was prepared to bootstrap some classes for a compatible stylesheet.

@jcubic
Copy link
Owner

jcubic commented Nov 21, 2020

Ok, did you tried this polyfill https://polyfill.io/v3/polyfill.min.js?features=Map service? There is quite a lot of code to polyfill Map object. The service is user-agent based and send JS only if needed. If it don't return anything for your blackberry browser then maybe you will need to report the bug to use the service.

I home this help, if this is fine then I will need only to document the use of Map object and suggest polyfill.

The main page is https://polyfill.io/v3/url-builder/ where you can use the polyfills you want.

@RussellAbraham
Copy link
Author

Ok, did you tried this polyfill https://polyfill.io/v3/polyfill.min.js?features=Map service? There is quite a lot of code to polyfill Map object. The service is user-agent based and send JS only if needed. If it don't return anything for your blackberry browser then maybe you will need to report the bug to use the service.

I home this help, if this is fine then I will need only to document the use of Map object and suggest polyfill.

The main page is https://polyfill.io/v3/url-builder/ where you can use the polyfills you want.

Thank you, the user agent is becoming a problem for this browser.

@jcubic
Copy link
Owner

jcubic commented Nov 30, 2020

Then probably you will need feature test kind of polyfill that's always included on the page.

jcubic added a commit that referenced this issue Dec 20, 2020
@jcubic
Copy link
Owner

jcubic commented Dec 20, 2020

Found shim for Map, but it says that implementation is slow, so I've made Map optiona,l since it was added to improve speed of rendering (used as cache when moving cursor when command is long and have syntax highlighting).

@jcubic jcubic added the resolved if issue is resolved, it will be open until merge with master label Dec 20, 2020
@RussellAbraham
Copy link
Author

RussellAbraham commented Dec 21, 2020

Great news. Yes it seems an iterable protocol is behind the implementation of Map done in C++.

While the syntax does not work 1:1 with modern javascript, the latest version of Backbone, provides the same constructs and is possibly more suitable for use in a legacy javascript environment.

@jcubic jcubic closed this as completed Jan 19, 2021
@jcubic
Copy link
Owner

jcubic commented Jan 19, 2021

releasing the fix in version 2.20.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug resolved if issue is resolved, it will be open until merge with master
Projects
None yet
Development

No branches or pull requests

2 participants