Conversation
st0nebreaker
added
the
team/content-platform
bretthayes
left a comment
bretthayes
left a comment
There was a problem hiding this comment.
Just one minor change. Thanks for porting this! 🚀
src/pages/hackathons.tsx
Outdated
| <div className="border-top border-light-9"> | ||
| <p className="pt-4"> | ||
| Get started with the{' '} | ||
| <a target="blank" href="https://docs.sourcegraph.com/admin/install/docker"> |
There was a problem hiding this comment.
Nothing to change here, but noting this for a fun fact.
MDN now states:
"Note: Setting target="_blank" on elements now implicitly provides the same rel behavior as setting rel="noopener" which does not set window.opener. See browser compatibility for support status."
Check this for older context. Typically we'd add either noopener or noreferrer but it looks like we don't have to anymore (considering which browsers we support via browserlist) since the spec has changed. Also, since this links out to a page that we own, we wouldn't use noreferrer here either since we'd want to track analytics.
There was a problem hiding this comment.
Interesting! So we can remove noreferrer now too? I removed any noopeners in the re-platform repo to fall in line with this
There was a problem hiding this comment.
Ah, I just noticed something! We actually have to change this lol. Our linting wasn't warning us because these should be set to target="_blank". So it looks like we have to set rel="noreferrer" here after all. Sorry for the confusion!
There was a problem hiding this comment.
Isn't it weird the lint doesn't work on blank, just _blank ? (blank vs _blank article). Should/can we adjust our lint rules to allow for noreferrer since it's an update? It seems like blank is a bit more handy than _blank to not cause duplicate tabs to open. What do you think?
There was a problem hiding this comment.
That's a good question! I would say let's go with _blank for UX and for default linting purposes instead of extending/overriding eslint. If a user wants to navigate around in the new tab that was created but also open another tab of the same link, then _blank would be preferred. Since this is linking to documentation, I would say it's easy for a user to get lost in the docs if we just brought them back to the link's href that had blank as the target.
Also, this is thought provoking with some interesting good/bad reasons/scenarios haha. It almost makes me want to omit all _blank/blank targets and let the user decide their linking behaviour with cmd/ctrl+click lol. Our target market is mostly technical folks but I thought this was lolz:
"Branding branding branding! Eyeballs baby. Metrics. ENGAGEMENT." 🤣
What do you think?
There was a problem hiding this comment.
Wow, interesting. I didn't know this was such an opinionated topic, hah! Lol @ engagement 😆 . I usually right-click or cmd-click to open a new tab when I want one anyways on <a>'s. 🤔 Maybe we should just remove all our _blank attributes on links then? I could see Marketing not loving that, to keep a user on our site, which albeit is a bad reason from that article 😬
I agree we shouldn't override the default lint too. I'll put up a change to adhere to our _blank attribute for now!
Good point for the docs site being a good reason for multiple duplicate tabs being open.
bretthayes
left a comment
bretthayes
left a comment
There was a problem hiding this comment.
Just a little bit of cleanup. Thanks for tackling some tech debt here around the site!! Woo! 🙌🏻
2 participants
Closes #92 by porting over the Hackathons page
Removes custom hubspot scss override to eliminate any confusion for hubspot styles. It was only being applied to the hackathons form, anyway.
Test
Navigate to
/hackathons