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.

Sign up for GitHub

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

Jump to bottom

feat: new hook useTitle #68

Merged
merged 3 commits into from
May 25, 2021
Merged

feat: new hook useTitle #68

merged 3 commits into from
May 25, 2021

Conversation

xobotyi
Copy link
Contributor

@xobotyi xobotyi commented May 16, 2021 •
edited
Loading

What new hook does?

Sets title of the page.

Checklist

  • Have you read contribution guideline?
  • Does the code have comments in hard-to-understand areas?
  • Is there an existing issue for this PR?
  • Have the files been linted and formatted?
  • Have the docs been updated?
  • Have the tests been added to cover new hook?
  • Have you run the tests locally to confirm they pass?

@codecov
Copy link

codecov bot commented May 16, 2021 •
edited
Loading

Codecov Report

Merging #68 (dc8e2f1) into master (6efe0f4) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master       #68   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           26        27    +1     
  Lines          355       368   +13     
  Branches        69        72    +3     
=========================================
+ Hits           355       368   +13
Impacted Files Coverage Δ
src/index.ts 100.00% <100.00%> (ø)
src/useTitle.ts 100.00% <100.00%> (ø)

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 6efe0f4...dc8e2f1. Read the comment docs.

@xobotyi xobotyi added the 🎂 new hook New hook added label May 18, 2021
@xobotyi xobotyi force-pushed the master branch 4 times, most recently from 20c19e3 to 665d454 Compare May 24, 2021 01:10
@xobotyi xobotyi marked this pull request as ready for review May 25, 2021 10:47
@xobotyi xobotyi requested a review from JoeDuncko May 25, 2021 10:47
@xobotyi xobotyi force-pushed the master branch 2 times, most recently from 915282c to 5369a94 Compare May 25, 2021 11:18
JoeDuncko
JoeDuncko reviewed May 25, 2021
View reviewed changes
Copy link
Contributor

@JoeDuncko JoeDuncko left a comment

Choose a reason for hiding this comment

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

The example doesn't seem to be working for me
Screen Recording 2021-05-25 at 4 15 07 PM

@JoeDuncko
Copy link
Contributor

Also, I think we should include a link to https://github.com/nfl/react-helmet in case they want to use something more powerful. Maybe we should come up with a formal way to include alternatives in our docs?

JoeDuncko
JoeDuncko requested changes May 25, 2021
View reviewed changes
Copy link
Contributor

@JoeDuncko JoeDuncko left a comment

Choose a reason for hiding this comment

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

See above

@xobotyi
Copy link
Contributor Author

xobotyi commented May 25, 2021 •
edited
Loading

The example doesn't seem to be working for me
Screen Recording 2021-05-25 at 4 15 07 PM

It actually works, buuuut, the document page is inside iframe =
Im digging into changing the docs generator to smth else.

About alternatives not from this package - not sure that we should share traffic, despite the fact that we almost have none %)
In general i have nothing against, just not now.

@JoeDuncko
Copy link
Contributor

That probably makes that a bad example lol. Anything we can do to make it better?

@xobotyi
Copy link
Contributor Author

xobotyi commented May 25, 2021

Nothing while using storybook, attempt to make it handle iframes will be quite bad idea, since react render flows are not aligned between parrent and child frame.

@xobotyi
Copy link
Contributor Author

xobotyi commented May 25, 2021

@JoeDuncko added notification about not working story.

@xobotyi xobotyi merged commit 84e4cbf into master May 25, 2021
@xobotyi xobotyi deleted the useTitle branch May 25, 2021 22:36
github-actions bot pushed a commit that referenced this pull request May 25, 2021
@semantic-release-bot
chore(release): 1.23.0 [skip ci]
ca56094 
# [1.23.0](v1.22.0...v1.23.0) (2021-05-25)

### Features

* new hook `useTitle` ([#68](#68)) ([84e4cbf](84e4cbf))
@xobotyi
Copy link
Contributor Author

xobotyi commented May 25, 2021

🎉 This PR is included in version 1.23.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@septs
Copy link

septs commented May 25, 2021

i want's the hook can rename to useDocumentTitle, thanks

@xobotyi
Copy link
Contributor Author

xobotyi commented May 26, 2021

Actually this is a good idea

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JoeDuncko JoeDuncko JoeDuncko requested changes

Assignees
No one assigned
Labels
🎂 new hook New hook added released
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@xobotyi @JoeDuncko @septs