-
Notifications
You must be signed in to change notification settings - Fork 162
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
Updates MediaQueryList to extend EventTarget #724
Conversation
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.
Thanks for the PR! Just a couple small changes, but looks good to me.
However, there seems to be an issue with the API reports. Try running prePR
again and committing them. If that doesn't work I will push a fix for you.
Co-authored-by: Arman Bilge <[email protected]>
This reverts commit f4938ee.
@armanbilge I have commited the changes you suggested, thank you for that. I've also reverted the commit regarding 2_12.txt. When I run
Maybe it fails to produce correct files because of the error? I am very much out of my scope here :/ |
Thanks! One more thing I realized: we should probably deprecate
Regarding the error ... I have a vague memory someone else on Windows had a similar problem. Let me see if I can find it. |
Right, someone had the exact same problem in #628 (comment). I don't know what causes it 😕 |
@armanbilge Maybe this is the issue somehow: lihaoyi/Scalatex#69 I am using |
Nice find, I bet that's it! |
It would seem so. I got it working both on Windows and on WSL2 now. On Windows I had to set this in the .gitconfig file:
on WSL2 it worked out of the box. Maybe place a message somewhere stating that having LF line endings is a must in order for prePR to complete or just suggesting to users to set I will drop these links here for future reference: |
@armanbilge I've added api-reports now, can you take a look at this PR and let me know if there is more for me to do? |
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.
LGTM, thank you very much!
I will update the contributor docs to mention the autocrlf = false
thing, thank you so much for getting to the bottom of it. It is pretty annoying, actually in the long-run we have to replace the documentation framework anyway as scalatex is no longer maintained.
Usually, I would consider taking such a task to replace the documentation because I like seeing nice documentation, but I am currently trying to finish 4 other projects of mine. Maybe once I am done with those I could circle back to this, can't promise. Do you know when the new release will happen or is there a way for me to use this branch in my project? I come from PHP background and there we can specify the branch in this exact case where you can't wait for a release 😊. Is there something like this in scala/sbt? |
No problem, appreciate all your contributions :)
It may be a while, since I just released 2.3.0 yesterday 😅 the current release rate has been every few months, but I don't mind doing it sooner once some more changes have accumulated. Sorry, bad timing! However, after I merge your PR there will be a snapshot release available which you can use immediately. I will merge your PR in a few days, just to give the other maintainers a chance to review it if they want. Snapshots are published here: https://oss.sonatype.org/content/repositories/snapshots/org/scala-js/ Until then, you can run There is one more option: since your change here is quite simple, in your own code you can just cast manually: prefersDark.asInstanceOf[EventTarget].addEventListener { ... |
It would seem that MediaQueryList was outdated, so I updated it according to the MDN: https://developer.mozilla.org/en-US/docs/Web/API/MediaQueryList
Changes
The main difference is that now MediaQueryList extends EventTarget which allows for
addEventListener
method and others. DeprecatedaddListener
andremoveListener
methods.I hope that I got the version number for @deprecated right.Issue
I could not get the compiler to compile with
addListener
because it expectedMediaQueryListListener
. Don't know if this has ever worked for someone. I am very new to scala and maybe I don't know how it was supposed to work.Before
would complain about function: Unit not being of type MediaQueryListListener.
After
should be good now.
sbt prePR
✔ (done)Not sure if this was my fault?
api-reports
❌ (what does this mean?)