Feat(banner) hide

[AugustinMauroy]

Description

A little while ago we discussed hiding the banner, and since then there have been no major objections, so I've opened this pr to implement it.

Related Issues

close #6292

Check List

• [x] I have read the Contributing Guidelines and made commit messages that follow the guideline.
• [x] I have run npm run format to ensure the code follows the style guide.
• [x] I have run npm run test to check if all tests are passing.
• [x] I have run npx turbo build to check if the website builds without errors.
• [x] I've covered new added functionality with unit tests if necessary.

[vercel[bot]]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
nodejs-org	✅ Ready (Inspect)	Visit Preview	Sep 26, 2024 7:29am

[AugustinMauroy]

also not OK, not to mention the whole hacky and weird UUID logic.

We can base the logic on dates and not a UIID.

I just overall don't feel confident with these changes and how much garbage we are adding just to have a single X button. The benefits are just not there.

I understand your point of view. I sent a message on slack to get a sample of opinions. And based on that:

• Either we abandon the changes.
• Or we find a clean solution to implement it.

Note that Caner had suggested using cookies, which has two advantages:

• the function for reading them is provided by next
• there's no more layout shift But we don't know what this means from a legal point of view ref: https://github.com/nodejs/nodejs.org/pull/7058#discussion_r1777421033

[ovflowd]

also not OK, not to mention the whole hacky and weird UUID logic.

We can base the logic on dates and not a UIID.

I just overall don't feel confident with these changes and how much garbage we are adding just to have a single X button. The benefits are just not there.

I understand your point of view. I sent a message on slack to get a sample of opinions. And based on that:

• Either we abandon the changes.

• Or we find a clean solution to implement it.

Note that Caner had suggested using cookies, which has two advantages:

• the function for reading them is provided by next

• there's no more layout shift

But we don't know what this means from a legal point of view

ref: https://github.com/nodejs/nodejs.org/pull/7058#discussion_r1777421033

Im fine with the feature -- Im just pretty much saying your code is not good, and I won't allow it to get merged on the state it is, sorry.

I might have more bandwidth in the future; But still I'm not sure about how much value we get from hiding banners. And this is such a small change that genuinely speaking bothering core collaborators to interfere here is overkill and unnecessary.

Some banners should just stay there, and I don't think you should make an UUID based on the dates. You could simply create a hash based on the banner entry itself (the json blob)

I also don't have the time at the moment to propose code changes or to coach you here. For me it is more valuable for Caner to take over your PR if we decide to move forward with this. Which I doubt we will.

Sorry, Augustin 😅

[ovflowd]

I forgot to comment regarding:

But we don't know what this means from a legal point of view

Just a FYI although it is a cookie, we could consider these to be functional 1st party cookies as they store user preferences.

Nothing is really done with them, I doubt on a GDPR standpoint anything needs to be done. Just so you know, the "legal concerns" you have are not limited to cookies, they also apply to localStorage.

[bmuenzenmeyer]

are we abandoning this for now then?

[AugustinMauroy]

are we abandoning this for now then?

I wanted to see how nextra and docausorus work. But I haven't taken the time for that yet.

[ovflowd]

I'm closing this for now, we can always circle back.