-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
impr: allow multiple funboxes with css (@notTamion) #6017
base: master
Are you sure you want to change the base?
impr: allow multiple funboxes with css (@notTamion) #6017
Conversation
You will need to go through the list of funboxes and check if they alter the same element. For example, now, you can enable mirror and upside down at the same time, but because they edit the same dom element, over overrides the other. Ill convert to draft. |
there are more combinations that i am not sure whether we should allow, for example: crt and spaceballs (having both enabled with still tilt the screen but no longer display the background image from spaceballs) |
Any combinations which change the behavior like that should be not allowed. |
bcd4759
to
422fb0f
Compare
Should have added validation for all incompatible funboxes now. ready for review |
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 fully like this approach. It creates what seem to be quite exclusive properties, but they should remain quite generic (worst offender of this is mirror, having its own property with the same name). For example, instead of movesText and rotatesText we could have css:typingTest, because they both modify that element. We might also want a new object key, like cssModifications (or something along those lines) to drop the css prefix.
Continuous integration check(s) failed. Please review the failing check's logs and make the necessary changes. |
Gonna put this on hold while #6063 is being finished up, Ill help you merge afterwards if needed. |
This PR is stale. Please trigger a re-run of the PR check action. |
Funbox refactor is now on master. Have a look and let me know if you need help. |
3930668
to
d9662ea
Compare
should be ported over properly i didn't really extensively test it as i am a bit handicapped right now cause my gpu decided to die yesterday so i can only use 1 monitor for the moment. |
This PR is stale. Please trigger a re-run of the PR check action. |
Description
Allows enabling multiple funboxes that have a stylesheet. Which funboxes should be deemed compatible with each other will need some discussion