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

Updated page title #439

Merged
merged 2 commits into from
Apr 20, 2022
Merged

Updated page title #439

merged 2 commits into from
Apr 20, 2022

Conversation

NLTN
Copy link
Contributor

@NLTN NLTN commented Apr 18, 2022

What caused the bug
Only the 404 error page has a title, whereas other pages don't. So, after being redirected to the 404 page, if the user clicks back or goes to other pages, the title doesn't change back to normal. It keeps saying "ACM at CSUF / 404" forever, no matter where the user goes.

What I changed to fix this issue

  • I added a page tile to the header of the following pages: About, Events, Node Buds, and Paths.

  • Also, I removed the title from the file "__layout.svelte" because every page has its own title from now on.

@vercel
Copy link

vercel bot commented Apr 18, 2022

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/ethanthatonekid/acm-csuf-site/5j83GSGuZDUS9KYC8X3CybZPSrLy
✅ Preview: https://acm-csuf-site-git-fix-296-ethanthatonekid.vercel.app

Copy link
Contributor

@jaasonw jaasonw left a comment

Choose a reason for hiding this comment

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

Well first thing's first, thank you for opening your first pull request with us, any help we get is greatly appreciated : )
Secondly, even though it's a small change, you should always write a brief description of what you changed and why (ex: to fix issue #xyz or something along those lines)
Third, is while it looks like this pull request was originally intended to fix #296, it doesn't look like I can reproduce the bug on my local copy of the main branch right now, so I guess it's technically fixed?
In which case, this looks alright to me as long as other people are on board with using | as a divider

@jaasonw jaasonw linked an issue Apr 19, 2022 that may be closed by this pull request
@NLTN
Copy link
Contributor Author

NLTN commented Apr 19, 2022

I just updated the description what I did.

One more thing I want to add. This is just a technical fix to solve the problem. Not about changing the naming conventions of the page titles.

@jaasonw
Copy link
Contributor

jaasonw commented Apr 20, 2022

image

Ohhhhhhh so thats how you reproduce it, in which case, the bug dies still exist in prod. Good catch and ty for updatng the description

@jaasonw jaasonw enabled auto-merge (squash) April 20, 2022 19:10
@vercel vercel bot temporarily deployed to Preview April 20, 2022 19:10 Inactive
@jaasonw jaasonw disabled auto-merge April 20, 2022 19:10
@jaasonw jaasonw merged commit 6bf8cc1 into main Apr 20, 2022
@jaasonw jaasonw deleted the fix/296 branch June 6, 2022 23:52
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.

Page title is incorrect after navigating
2 participants