-
-
Notifications
You must be signed in to change notification settings - Fork 380
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: update title of question page #3443
Conversation
Please let me know if a different workaround was required, or a different approach was expected! I'd be happy to work on that as well! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this @ADTmux. I think it's better to set it on QuestionPage
though and I'd love to see it tested in the cypress test for questions/read
.
hey @benfurber , tested on Cypress and did the requested change. Please let me know what next is required! :) |
@ADTmux Sorry to request another change on a tiny commit(!) but I think it should be set during the useeffect rather than the render. Let's go with just after The branch is also failing the linting right now. That's easiest to fix by running |
Hi @benfurber could you test it now? |
Awesome. Thanks @ADTmux. So I think that updating the commit messages to our style will fix the linter plus a rebase with master and we're good to go. |
dfd337a
to
67dc9a9
Compare
@benfurber Done! |
Hey @ADTmux. Thanks for making that change. Looks like you've pulled in some unrelated (and wrong) changes too? The stuff |
Just checking in @ADTmux. Have you seen my last comment? :) |
Closing this as I've sorted it in #3623 |
PR Checklist
PR Type
Description
Clicking a question's title changes the name of the tab to reflect that question.
Git Issues
Closes #3409