Skip to content

docs: overhaul branding and add new logo #6147

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

Merged
merged 13 commits into from
Dec 8, 2022

Conversation

JoshuaKGoldberg
Copy link
Member

@JoshuaKGoldberg JoshuaKGoldberg commented Dec 1, 2022

PR Checklist

Overview

  • Brings in the new logo we settled on internally
  • Standardizes the name as "typescript-eslint", not "TypeScript ESLint" or "TypeScript-ESLint"
  • Standardizes the slogan:

TypeScript-ESLint logo from the PR

@nx-cloud
Copy link

nx-cloud bot commented Dec 1, 2022

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 4141f71. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch


✅ Successfully ran 47 targets

Sent with 💌 from NxCloud.

@typescript-eslint
Copy link
Contributor

Thanks for the PR, @JoshuaKGoldberg!

typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community.

The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately.

Thanks again!


🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint.

@netlify
Copy link

netlify bot commented Dec 1, 2022

Deploy Preview for typescript-eslint ready!

Name Link
🔨 Latest commit 4141f71
🔍 Latest deploy log https://app.netlify.com/sites/typescript-eslint/deploys/63921f4f4c36110009856095
😎 Deploy Preview https://deploy-preview-6147--typescript-eslint.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@JoshuaKGoldberg JoshuaKGoldberg marked this pull request as ready for review December 2, 2022 01:12
Copy link
Member

@JamesHenry JamesHenry left a comment

Choose a reason for hiding this comment

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

I love the look of the final form of the new logo now, great stuff!

Uses that instead of which - which I barely 51% confident as [Grammarly which-vs-that explainer]

Wow this explainer is actually really good and I genuinely wasn't familiar with this technicality between them, I think I'd always based it on "feel".

Standardizes the name as "TypeScript-ESLint", not "typescript-eslint" or "TypeScript ESLint"

This one really caught me off guard, it would have best to have discussed this first, I haven't seen it mentioned in our chats at all.

The name of the project is typescript-eslint and it always has been. That's the reason why this PR has had to make so many changes.

The onus would be on any proposed name change to justify why it would be better, but nevertheless here are some reasons why typescript-eslint has always been the choice:

  • It simply has to be written as typescript-eslint in so many important contexts

    • domain name
    • npm
    • github
      ...so having an alternate casing of the same words is of questionable value and just creates noise.
  • TypeScript is how the TypeScript project is written, ESLint is how the ESLint project is written, so combining them together with a dash actually makes it less clear that a separate unique project is being referenced. Having typescript-eslint helps differentiate it in a sentence.

Please let's discuss these things going forward to avoid confusion 🙏

Again, love the new logo and thanks for improving my grammar!

@JoshuaKGoldberg
Copy link
Member Author

Please let's discuss these things going forward to avoid confusion 🙏

I was hoping this PR would be the discussion! 😄

Your points make sense - I'll switch this to prefer typescript-eslint, and add that explainer in the docs. Thanks!

@Josh-Cena
Copy link
Member

  • domain name
  • npm
  • github

These places all use all-lowercase by convention, but that might not be the canonical name of the product. ESLint uses all-lowercase in these three places as well; TypeScript uses all-lower in the former two, but that's only because Microsoft is weird and they like using CamelCase even in GitHub repos.

"ESLint",
"typescript-eslint"
"typescript-eslint",
"TypeScript"
Copy link
Member Author

Choose a reason for hiding this comment

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

No actual changes here, just sorting. It happens automatically sometimes anyway.

@@ -19,6 +19,7 @@
}

.sponsorsTier img {
background: white;
Copy link
Member Author

Choose a reason for hiding this comment

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

Tangentially related: I noticed sponsor logos such as Nrwl weren't showing up in dark mode. Branding!

@JoshuaKGoldberg
Copy link
Member Author


<--- Last few GCs --->

[1980:0x6b6a320]   181736 ms: Scavenge 2010.7 (2081.0) -> 2006.1 (2081.0) MB, 20.0 / 0.0 ms  (average mu = 0.515, current mu = 0.414) allocation failure; 
[1980:0x6b6a320]   181765 ms: Scavenge 2011.7 (2081.0) -> 2007.7 (2081.0) MB, 8.2 / 0.0 ms  (average mu = 0.515, current mu = 0.414) allocation failure; 
[1980:0x6b6a320]   181823 ms: Scavenge 2013.9 (2081.0) -> 2009.6 (2097.0) MB, 11.6 / 0.0 ms  (average mu = 0.515, current mu = 0.414) allocation failure; 


<--- JS stacktrace --->

FATAL ERROR: Reached heap limit Allocation failed - JavaScript heap out of memory
 1: 0xb6e500 node::Abort() [/opt/hostedtoolcache/node/18.12.1/x64/bin/node]

Ohhh boy!

@JamesHenry
Copy link
Member

JamesHenry commented Dec 3, 2022

Ok perfect, it's looking good!

Please could we update these two (the navbar and the hero):

image

Maybe for the hero we could add the logo in there to accompany the typescript-eslint? Or even have it instead of the words? 👀 (given it's already in the upper right)

I'm totally fine if we want to bump a hero update to a follow up though and spend some more time on it...

Other minor thing on the home page:

image

I think that's our old logo rather than the actual TS logo (i.e. I don't think that's their color)

@JamesHenry
Copy link
Member

I know not caused by this PR, but seems strange that the section get started buttons are skewed to the left?

image

@JoshuaKGoldberg
Copy link
Member Author

for the hero we could add the logo in there

Hmm yeah +1. I think I'd like to do that as a followup. There are a few different treatments we could try and I'm not solid on which one is best. 🤔

our old logo rather than the actual TS logo

Ha! Yes. Changing.

skewed

Blurgh yeah I haven't been able to figure this one out: #6081. Thanks for the bump - pinging in that issue.

@JamesHenry
Copy link
Member

How come this is just a black shape? packages/website/static/img/favicon/safari-pinned-tab.svg

@@ -66,7 +66,7 @@ const themeConfig: ThemeCommonConfig & AlgoliaThemeConfig = {
],
image: 'img/logo-twitter-card.png',
navbar: {
title: 'TypeScript ESLint',
title: 'typescript-eslint',
Copy link
Member Author

Choose a reason for hiding this comment

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

@JamesHenry @Josh-Cena this is why I'd gone towards solidifying as TypeScript-ESLint at first. IMO it looks a little odd to have the project name be lower-kebab-case on the homepage:

Screenshot of the deploy preview homepage with lower-kebab-case 'typescript-eslint' as the title

My logic was:

  1. I think it's good to keep the title casing & general phrasing consistent where possible
  2. For a project homepage, it makes sense to keep things in title case
  3. The hyphen is useful for showing it's a single project
  4. Therefore, TypeScript-ESLint is the logical conclusion

Comment reference to counterpoints: #6147 (review)

Thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

Choose a reason for hiding this comment

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

We can make the hero prettier in general in a follow up as we mentioned

Copy link
Member

Choose a reason for hiding this comment

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

webpack and many others have the same approach, but they mostly combine the word with the logo, we can do the same

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah I've started playing with this locally and really like putting that logo there. I'll throw up a few examples soon 😄

JamesHenry
JamesHenry previously approved these changes Dec 3, 2022
@JamesHenry
Copy link
Member

What is jest seeing here? 🤔

image

If I do a copy of the first line and "find on page" I get 2 results, the 2 lines appear identical in the github diff.

Maybe indentation or even invisible unicode chars?

JamesHenry
JamesHenry previously approved these changes Dec 4, 2022
@Josh-Cena
Copy link
Member

Josh-Cena commented Dec 5, 2022

FWIW, I like either TypeScript-ESLint or TypeScript ESLint much better than all lower, simply because we've used it for ever, and it feels confusing to switch (also considering that users may have to expand their spell checker dictionary!)... The only motivation is because there are contexts where lowercase is inevitable, but that doesn't feel convincing enough to me, because that in essence means all projects need to be lowercase, which isn't reality.

@JamesHenry
Copy link
Member

@Josh-Cena The project's name was never in scope for changing as part of this work. One of the few luxuries afforded to me as the project's creator is that I get to decide what it is called :)

we've used it for ever

Likewise with this... it simply isn't the case. I called the project typescript-eslint, so very literally that is what it has been called "for ever"

bradzacher
bradzacher previously approved these changes Dec 5, 2022
@JoshuaKGoldberg JoshuaKGoldberg dismissed stale reviews from bradzacher and JamesHenry via 10efa72 December 8, 2022 17:29
@JoshuaKGoldberg JoshuaKGoldberg merged commit 47241bb into typescript-eslint:main Dec 8, 2022
@JoshuaKGoldberg JoshuaKGoldberg deleted the branding branch December 8, 2022 20:08
@jerone jerone mentioned this pull request Dec 14, 2022
3 tasks
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Docs: Branding guide
4 participants