Skip to content
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

Fix titleTemplate bug with multiple title children components #351

Merged
merged 2 commits into from
Feb 26, 2018
Merged

Fix titleTemplate bug with multiple title children components #351

merged 2 commits into from
Feb 26, 2018

Conversation

joshgummersall
Copy link
Contributor

@joshgummersall joshgummersall commented Feb 9, 2018

I discovered a bug when using titleTemplate with a component that essentially renders a <title> with multiple child components (see the test case for an accurate simulation of this use case). This pull request fixes that behavior by calling innermostTitle.join('') when doing the template replacement for innermostTitles that are arrays.

Josh Gummersall added 2 commits February 9, 2018 12:19
Regexp replacement with a function will call .toString() on the
replacement candidate. That would include extra commas in page titles
where the <title> component had multiple children.
@codecov
Copy link

codecov bot commented Feb 9, 2018

Codecov Report

Merging #351 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #351      +/-   ##
=========================================
+ Coverage   97.59%   97.6%   +<.01%     
=========================================
  Files           3       3              
  Lines         291     292       +1     
=========================================
+ Hits          284     285       +1     
  Misses          7       7
Impacted Files Coverage Δ
src/HelmetUtils.js 96.87% <100%> (+0.01%) ⬆️
src/Helmet.js 100% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 86276f3...c4616e0. Read the comment docs.

@jamsea
Copy link
Contributor

jamsea commented Feb 26, 2018

Looks good to me @joshgummersall !

@jamsea jamsea merged commit 6e65388 into nfl:master Feb 26, 2018
@joshgummersall joshgummersall deleted the fix-title-children-title-template branch February 26, 2018 21:18
vcarl added a commit to vcarl/react-helmet that referenced this pull request May 21, 2019
vcarl added a commit to vcarl/react-helmet that referenced this pull request May 21, 2019
vcarl added a commit to vcarl/react-helmet that referenced this pull request May 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants