change how we title-case

[gkchestertron]

Overview

Does this PR close an existing issue?

No PR should be opened without opening an issue first. Any change needs to be discussed before proceeding.

Summary

Provide a general summary of the issue addressed in the title above

Issue Number(s)

Which issue(s) does this PR address?

Put Closes #XXXX below to auto-close the issue that this PR addresses:

• Closes #

Screenshots or recordings

Please provide screenshots or recordings if this PR is modifying the visual UI or UX.

Checklist

• I have added tests that prove my fix is effective or that my feature works
• I have evaluated if the README.md documentation needs to be updated
• I have evaluated if the /tests/dummy/ app needs to be modified
• I have evaluated if DocBlock headers needed to be added or updated
• I have verified that lint and tests pass locally with my changes
• If a fork of a dependent package had to be made to address the issue this PR closes:
  • I noted in the fork's README.md the reason the fork was created
  • I have opened an upstream issue detailing what was deficient about the dependency
  • I have opened an upstream PR addressing this deficiency
  • I have opened an issue in this repository to track this PR and schedule the removal of the usage of the fork

Semver

This project uses semver, please check the scope of this PR:

• #none#
• #patch#
• #minor#
• #major#

Examples:

• NONE
  • README.md changes
  • test additions
  • changes to files that are not used by a consuming application (.travis.yml, .gitignore, etc)
• PATCH
  • backwards-compatible bug fix
    • nothing about how to use the code has changed
    • nothing about the outcome of the code has changed (though it likely corrected it)
  • changes to demo app (/tests/dummy/)
• MINOR
  • adding functionality in a backwards-compatible manner
    • nothing about how used to use the code has changed but using it in a new way will do new things
    • nothing about the outcome of the code has changed without having to first use it in a new way
    • addition of new CSS selectors
    • addition of new ember-hook selectors
• MAJOR
  • incompatible API change
    • using the code how used to will cease working
    • using the code how used to will have a different outcome
    • any changes to CSS selector names
    • any removal of CSS selectors
    • any changes to ember-hook selectors
    • possibly changes to test helpers (depends on the changes made)
  • any changes to the dependencies entry in the package.json file

CHANGELOG

• Changed to only title-case first word in a section

[EWhite613]

If it's only Capitalizing the first word can we avoid the map, otherwise just looping through all the words for no reason?

[gkchestertron]

yup - thanks

[notmessenger]

My preference leans towards frost-page-title which is the convention seen most often in the Ember ecosystem (ember-cli-mirage, ember-cli-deploy, ember-simple-auth, etc, as well as our own https://github.com/ciena-frost/ember-frost-core/blob/master/docs/frost-icons.md#addon-icon-packs). Which do you prefer @juwara0 ?

[gkchestertron]

I like frost-page-title, but as its own namespace

[notmessenger]

Oh sorry, yes, I agree. I just wasn't clear.

{
  'frost-page-title': {
    default: '',
    delimiter: ''
}

my example doesn't use title- as a preface which is cleaner, but not sure if default makes sense then - maybe title?

We're on the same page so whatever you roll with.

[gkchestertron]

Yeah - I think that stays defaultTitle. I don't honestly care any which way - but it is nice to get it into a namespace, as we may have other options to add later.

[notmessenger]

@overview at the top of addon/services/frost-page-title.js should be on its own line

[notmessenger]

Replace https://github.com/gkchestertron/ember-frost-page-title/blob/c09f505e9d9d333927655d775c83e328115cd022/addon/services/frost-page-title.js#L64 with this.getWithDefault('delimiter', '|')

[notmessenger]

Don't know if you'd want to replace https://github.com/gkchestertron/ember-frost-page-title/blob/c09f505e9d9d333927655d775c83e328115cd022/addon/services/frost-page-title.js#L62-L64 with

const {defaultTitle, handlers, delimiter} = this.getProperties('defaultTitle', '_handlers', 'delimter') or if not worth it because then have to massage the delimiter value anyways.

[notmessenger]

What are your thoughts on

document.title = sections.length ? sections.join( ${delimiter} ) : defaultTitle

for https://github.com/gkchestertron/ember-frost-page-title/blob/c09f505e9d9d333927655d775c83e328115cd022/addon/services/frost-page-title.js#L70-L74 ?

I could go either way (though slightly prefer the ternary more)

[gkchestertron]

I like the ternary. I generally avoid them unless they actually seem clearer and are unlikely to be added to. They can get unreadable quick. But this seems like a fine case for it.

[notmessenger]

@gkchestertron remember to coordinate with @juwara0 or myself before merging this PR (temporary requirement)