-
Notifications
You must be signed in to change notification settings - Fork 11.9k
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
Update supported browser section of documentation #4818
Conversation
docs/developers/README.md
Outdated
* Firefox | ||
* Internet Explorer 11 | ||
* Edge | ||
* Safari | ||
|
||
Browser support for the canvas element is available in all modern & major mobile browsers. [CanIUse](http://caniuse.com/#feat=canvas) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you still want this line since you removed the mention of canvas above?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about that. Wasn't sure if it was best to leave it in or not. I'm happy to change it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about combining the lines into:
Chart.js works in any browser with canvas support, which is any modern & major browser including:
- Chrome
- Firefox
- Internet Explorer 11
- Edge
- Safari
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I take it back. I would remove any mention of canvas. Canvas is supported in IE9 and above, but we only support IE11+, so it's confusing to talk about the browser versions that support canvas
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree, we should remove the part about canvas compatibility since we use other features that are not supported in all browsers. Ideally, we should also decide about minimal versions for all browsers, not only IE 11+.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think us testing the latest versions is fine, but we also don't want to say 'Chrome' and have some using an ancient version that doesn't work. I was thinking we could roll these supported version as the 2nd oldest Firefox ESR and then whatever Chrome version was available then. How does that sound?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know that we'd come back to update these versions whenever new browser versions are released. What if we said:
- Chrome stable
- Firefox stable and ESR
- Internet Explorer 11
- Edge stable and other most recent supported version
- Safari stable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm ok with that. @simonbrunel are you?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a commitment with our users (not - only - contributors) that we will make sure it doesn't break for the listed versions and if it breaks, that will be considered as a bug and not a "won't fix, because it works in the latest stable version". Being vague on this commitment will not make Chart.js more trustful.
Having precise supported versions actually help us to be consistent and prevent debate on PR introducing new browser features: let's imagine we are not supporting IE11 anymore, then what features I'm allowed to use in my contributions? Is it ok to break all versions before "stable" as soon it's released?
I'm fine for testing only stable versions when contributing until we have an automated way to test all supported browsers. Anyway, with versions picked by @etimberg, we should be guaranteed that if it breaks for Chrome/Firefox, it will break for IE11 first. At this point, since IE11 is the one which defines the features scope, I prefer to commit on locked versions for other browsers as you did in your last changes.
https://www.highcharts.com/docs/getting-started/compatibility
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I agree that it's a commitment primarily with our users.
However, I don't think it's vague to say that we will support only the latest version of Chrome. I think that's very clear.
In the case of dropping IE11, what I'm suggesting is that we would support the latest version of Edge and the previous stable version. That would be Edge 38.14393 released over 1 year ago.
What I'm wary of is making a commitment to our users that we don't have a strong way of keeping. If we can't test supported browsers then how do we know we aren't breaking them?
@etimberg just a reminder about this PR |
Updates the documentation per #4742