Skip to content

Conversation

MichaelArestad
Copy link
Contributor

@MichaelArestad MichaelArestad commented Jan 26, 2017

Changes proposed in this Pull Request:

  • removes title case in settings headings

Testing instructions:

  • Go to Settings and check all the headings to make sure they are not using Title Case

@zinigor @eliorivero or @georgestephanis I couldn't figure out how to change it for all of them. Can one of you fix these:

  • Custom Content Types > Custom content types
  • Related Posts > Related posts
  • Site Verification > Site verification

@MichaelArestad MichaelArestad self-assigned this Jan 26, 2017
@MichaelArestad MichaelArestad added [Status] In Progress [Type] Bug When a feature is broken and / or not performing as intended labels Jan 26, 2017
@jeherve jeherve added the Admin Page React-powered dashboard under the Jetpack menu label Jan 26, 2017
@jeherve jeherve added this to the 2/17 - February milestone Jan 26, 2017
@eliorivero
Copy link
Contributor

Those are using the module name as is defined in the module file header. Would you prefer to update the module file or add a custom, no capitalized, string in JS?

Note that if the module file header, for example, modules/related-posts.php is updated, the changes are not instantaneous because it's read from modules/module-headings.php which is updated after a yarn build (but can be also manually updated for testing purposes).

@eliorivero eliorivero added the [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. label Jan 26, 2017
@beaulebens
Copy link
Member

From what I can tell, every heading in Calypso uses title case, while every option/label uses sentence case. I think we should be matching that, which means we should be ensuring that headings are title case in Jetpack.

@beaulebens
Copy link
Member

beaulebens commented Jan 26, 2017

The places that I found which do not already have Title Case (e.g. which would need to be updated) are;

  • Security > Backups and security scanning
  • Security > Brute force protection
  • Security > WordPress.com log in
  • Traffic > Site stats
  • Writing > Theme enhancements

Also cleaned up a bunch of linting errors.
@beaulebens beaulebens changed the title Removes title case from headings in settings Use Title Case consistently in card titles, to match Calypso Jan 26, 2017
import SettingsCard from 'components/settings-card';
import SettingsGroup from 'components/settings-group';
import { SettingsCard } from 'components/settings-card';
import { SettingsGroup } from 'components/settings-group';
Copy link
Contributor

@eliorivero eliorivero Jan 27, 2017

Choose a reason for hiding this comment

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

@beaulebens this change was causing a major JS error on Writing tab. I've reverted it to the way I wrote it, without the curly braces since the imported component is the default exported.

Copy link
Member

Choose a reason for hiding this comment

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

Ah cool. I made that change when resolving a conflict, just to match the others. 👍

@eliorivero eliorivero force-pushed the fix/settings-headings branch from 72a23e4 to c33ffc8 Compare January 27, 2017 13:17
@eliorivero eliorivero force-pushed the fix/settings-headings branch from c33ffc8 to cc26a45 Compare January 27, 2017 13:17
@eliorivero eliorivero removed the [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. label Jan 27, 2017
@MichaelArestad
Copy link
Contributor Author

It looks like after a discussion in Slack, we should be using sentence case for all of these headings as per our branding guidelines: https://dotcombrand.wordpress.com/brand-voice/#style

Calypso will need to be updated eventually to match.

@eliorivero eliorivero modified the milestones: 2/17 - February, Settings UI Jan 30, 2017
@eliorivero
Copy link
Contributor

Closing this in favor of #6394 since this one was doing title case and we'll be doing sentence case plus it has a bunch of conflicts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Admin Page React-powered dashboard under the Jetpack menu [Type] Bug When a feature is broken and / or not performing as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants