-
Notifications
You must be signed in to change notification settings - Fork 25
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
Initial PR for removing top Explain This button for feedback #3256
Conversation
Some notes:
|
@misaugstad The new commit reverted the random changes and also removed the entire bar, without changing anything from the mobile view! |
Great! Did we need to keep all of the text for the Validate page, or is there anything from there that we can remove? |
Is there specific text that you are looking at to remove? It looks good to me, and it seems there's no redundant code from the changes I've made and the functions I've deleted. |
…walk/SidewalkWebpage into 3250-remove-top-explain-this
…into 3250-remove-top-explain-this
…walk/SidewalkWebpage into 3250-remove-top-explain-this
…Webpage into 3250-remove-top-explain-this
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.
Okay I didn't exhaustively go through everything, but I noted at least a few examples where translations/code still need to be removed, and one spot where some CSS was removed that shouldn't have been!
Can you go through everything again with a fine-toothed comb?
…walk/SidewalkWebpage into 3250-remove-top-explain-this
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.
Okay I cleaned up a few remaining things so that we could move on to something else!
Resolves #3250
PR for removing the top bar "Explain This" issue.
Before:
After: