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

feat: modified/added meta tags with new content #10652

Merged
merged 3 commits into from
Jan 14, 2025
Merged

Conversation

jordanh
Copy link
Contributor

@jordanh jordanh commented Jan 9, 2025

Description

Closes #9407

Testing scenarios

  • Navigate to Sign-Up Page

    • Note and proofread new meta
  • Navigate to Create Account Page

    • Note and proofread new meta
  • Share an invitation link

    • Note and proofread new meta
  • Share any other URL

    • Note and proofread new meta

Final checklist

  • I checked the code review guidelines
  • I have added Metrics Representative as reviewer(s) if my PR invovles metrics/data/analytics related changes
  • I have performed a self-review of my code, the same way I'd do it for any other team member
  • I have tested all cases I listed in the testing scenarios and I haven't found any issues or regressions
  • Whenever I took a non-obvious choice I added a comment explaining why I did it this way
  • I added the label Skip Maintainer Review Indicating the PR only requires reviewer review and can be merged right after it's approved if the PR introduces only minor changes, does not contain any architectural changes or does not introduce any new patterns and I think one review is sufficient'
  • PR title is human readable and could be used in changelog

Copy link
Contributor

@Dschoordsch Dschoordsch left a comment

Choose a reason for hiding this comment

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

I'm not sure what the goal of some of the changes is. If we want to have a nice preview in Slack etc. for different routes, I think there is no way around serving different versions of index.html based on route, so we could have 1 generic, 1 for mass invite and 1 for team invite?
Let me know if I'm just missing something here.

packages/client/components/GenericAuthentication.tsx Outdated Show resolved Hide resolved
devTemplate.html Outdated Show resolved Hide resolved
devTemplate.html Outdated Show resolved Hide resolved
@jordanh
Copy link
Contributor Author

jordanh commented Jan 14, 2025

I'm not sure what the goal of some of the changes is. If we want to have a nice preview in Slack etc. for different routes, I think there is no way around serving different versions of index.html based on route, so we could have 1 generic, 1 for mass invite and 1 for team invite? Let me know if I'm just missing something here.

I was following work from here: https://www.notion.so/parabol/c8419b4417324a8cae36570631e62beb?v=c18a873d44014bc3ab24eb850e325771&pvs=4

Your comments make sense to me.

I am going to adjust the goal of this work to simply update the meta text to broaden our intent slightly beyond agile for unauthenticated routes and back the rest of the changes out.

@jordanh
Copy link
Contributor Author

jordanh commented Jan 14, 2025

@Dschoordsch thank you for the review! I updated based upon your comments

@Dschoordsch
Copy link
Contributor

@jordanh Please next time press the "Re-request review" button as I might miss mentions.

template.html Outdated Show resolved Hide resolved
@jordanh jordanh merged commit 9501494 into master Jan 14, 2025
5 checks passed
@jordanh jordanh deleted the feat/update-meta branch January 14, 2025 22:29
Copy link

sentry-io bot commented Jan 22, 2025

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ Error: Rendered more hooks than during the previous render. useDocumentTitle(packages/client/hooks/useDocum... View Issue

Did you find this useful? React with a 👍 or 👎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update meta descriptions and titles
2 participants