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

feat: add static Christmas theme #374

Merged
merged 2 commits into from
Dec 21, 2024

Conversation

acolombier
Copy link
Member

Adding a static Christmas theme (logo+snow fall), align with the initiative in mixxxdj/mixxx#14027

This should be revert after the 6th of Jan.

Santa hat license to be approved.

@Holzhaus
Copy link
Member

Can you rebase on latest website branch? I think that should probably fix the build errors.

@acolombier acolombier force-pushed the feat/christmas-vibe branch 2 times, most recently from 5bb2547 to 497dbbd Compare December 17, 2024 00:28
@acolombier
Copy link
Member Author

Ah I had just fixed it 😅

@acolombier
Copy link
Member Author

acolombier commented Dec 17, 2024

@Holzhaus I have pushed my fix as it implied to use lastest version rather than reverting to a fairly old version of PyYAML - what your thought about this? Would you like to resync your fork?

Edit: looks like there is more work to support newer version of PyYAML, but we should really look into it as the current version is quite vulnerable

@Holzhaus
Copy link
Member

I agree we should try to use a newer version, but I'd say the risk is rather low since we do not parse zntrust input, only stuff from this repo. And the Netlify build container is also isolated.

Maybe we can switch to ruamel.yaml? IIRC it also faster.

@Holzhaus
Copy link
Member

Re: This PR: the Mixxx logo looks slightly transformed. Aspect ratio seems to have changed, it's less wide but has roughly the same height compared to the logo without this change. But doubt someone notices without direct comparison ;)

Looks cool, thanks!

@acolombier
Copy link
Member Author

acolombier commented Dec 17, 2024

My fear is for someone to submit a PR to this repo, which would triggers the CI and potentially steal token to perform privilege escalation, similar to Ultralytics event last week, The risk is very low, and I believe first time contributor need CI approval so we should be fine. Still worth not letting out for too long :)

Maybe we can switch to ruamel.yaml? IIRC it also faster.

I've never used this one before, but no objection! Anything maintained would be good to me.

Looks cool, thanks!

Glad you like it! Just need the 👍 on the hat used due to potential license risks

@daschuer
Copy link
Member

Just need the 👍 on the hat used due to potential license risks

Mmm that could be really an issue, can't we just draw our own?

@Eve00000
Copy link
Contributor

Very nice. I appreciate the work done.
It's fun and creates a christmas-community-spirit.

@acolombier
Copy link
Member Author

can't we just draw our own?

I'll see what I can do!

@acolombier
Copy link
Member Author

Updated with @peineduper's asset rework as well!

image

We can just press merge as soon as we want to turn on the XMas Mixxx mood

@Eve00000
Copy link
Contributor

We can just press merge as soon as we want to turn on the XMas Mixxx mood

will there be some snowflakes behind the blogpost as well?

@acolombier
Copy link
Member Author

Yes, you can see a live preview here

@Eve00000
Copy link
Contributor

Yes, you can see a live preview here

ah ok, sorry, I thought it would only be in the upper part as in the preview, because when I click on 'news' there are no snowflakes.
I like this 'temporary theme' a lot. ❤️ 🎅

@acolombier
Copy link
Member Author

I'm glad yo hear you do like it!
Oh sorry, I misread your comment about snowflakes. I wouldn't mind adding the snowflake falls on the news page as well! Just keen to hear what the rest of the team think about it?

@acolombier
Copy link
Member Author

Merge?

Copy link
Member

@Holzhaus Holzhaus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel free to hit merge yourself when you think the time fits.

@daschuer
Copy link
Member

I am already in Christmas mood, with all our efforts. Thank you.🎅🎄🌠

@Eve00000
Copy link
Contributor

❄️ ❄️ ❄️ 🎁 ☃️ I ❄️ am ❄️ getting ❄️ in ❄️ the ❄️ mood ❄️ as ❄️ well 🎅🎄 🎁 ☃️ ❄️ ❄️ ❄️ ☃️ 🎁
thank you for creating this

@acolombier
Copy link
Member Author

Thanks for starting the Christmas train @daschuer ! Do you want to merge the PR now? I don't seem to have the permission to do so.

@daschuer
Copy link
Member

@acolombier I have just granted your the access to the website and manual

@daschuer daschuer merged commit 8aa31a8 into mixxxdj:website Dec 21, 2024
6 checks passed
@fwcd
Copy link
Member

fwcd commented Dec 23, 2024

Love this, looks awesome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants