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

impr: allow multiple funboxes with css (@notTamion) #6017

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

notTamion
Copy link
Contributor

Description

Allows enabling multiple funboxes that have a stylesheet. Which funboxes should be deemed compatible with each other will need some discussion

@monkeytypegeorge monkeytypegeorge added the frontend User interface or web stuff label Nov 7, 2024
@Miodec
Copy link
Member

Miodec commented Nov 8, 2024

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.

@Miodec Miodec marked this pull request as draft November 8, 2024 14:19
@notTamion
Copy link
Contributor Author

notTamion commented Nov 8, 2024

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)

@Miodec
Copy link
Member

Miodec commented Nov 8, 2024

Any combinations which change the behavior like that should be not allowed.

@notTamion notTamion force-pushed the allow-multiple-css-funboxes branch from bcd4759 to 422fb0f Compare November 14, 2024 19:43
@notTamion
Copy link
Contributor Author

Should have added validation for all incompatible funboxes now. ready for review

@notTamion notTamion marked this pull request as ready for review November 14, 2024 19:44
@github-actions github-actions bot added the waiting for review Pull requests that require a review before continuing label Nov 14, 2024
Copy link
Member

@Miodec Miodec left a 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.

@Miodec Miodec added waiting for update Pull requests or issues that require changes/comments before continuing and removed waiting for review Pull requests that require a review before continuing labels Nov 18, 2024
@github-actions github-actions bot removed the waiting for update Pull requests or issues that require changes/comments before continuing label Nov 20, 2024
Copy link
Contributor

Continuous integration check(s) failed. Please review the failing check's logs and make the necessary changes.

@monkeytypegeorge monkeytypegeorge added the assets Languages, themes, layouts, etc. label Nov 20, 2024
@Miodec
Copy link
Member

Miodec commented Nov 21, 2024

Gonna put this on hold while #6063 is being finished up, Ill help you merge afterwards if needed.

@Miodec Miodec added the on hold Pull request on hold label Nov 21, 2024
Copy link
Contributor

This PR is stale. Please trigger a re-run of the PR check action.

@github-actions github-actions bot added the Stale Has not been updated in a while label Nov 28, 2024
@Miodec
Copy link
Member

Miodec commented Dec 4, 2024

Funbox refactor is now on master. Have a look and let me know if you need help.

@Miodec Miodec added waiting for update Pull requests or issues that require changes/comments before continuing and removed on hold Pull request on hold labels Dec 4, 2024
@notTamion notTamion force-pushed the allow-multiple-css-funboxes branch from 3930668 to d9662ea Compare December 4, 2024 16:33
@monkeytypegeorge monkeytypegeorge added the packages Changes in local packages label Dec 4, 2024
@github-actions github-actions bot removed the waiting for update Pull requests or issues that require changes/comments before continuing label Dec 4, 2024
@notTamion
Copy link
Contributor Author

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.

@github-actions github-actions bot removed the Stale Has not been updated in a while label Dec 4, 2024
Copy link
Contributor

This PR is stale. Please trigger a re-run of the PR check action.

@github-actions github-actions bot added the Stale Has not been updated in a while label Dec 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assets Languages, themes, layouts, etc. frontend User interface or web stuff packages Changes in local packages Stale Has not been updated in a while
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants