-
-
Notifications
You must be signed in to change notification settings - Fork 427
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
Fix/815 #927
base: master
Are you sure you want to change the base?
Fix/815 #927
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
Thank you!
I love the idea of adding it to the bottom nav bar 💅!
About the icon, where did you get it? Can it be used in terms of copyrights?
src/Me/Navigation/Navbar.tsx
Outdated
<NavItemDecoration | ||
className={classNames({ active: router.pathname === to })} | ||
> | ||
<button onClick={toggleChat}> |
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.
Since you pass onClick
, did you mean
<button onClick={toggleChat}> | |
<button onClick={onClick}> |
?
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.
ive done this
src/Me/Navigation/Navbar.tsx
Outdated
useEffect(() => { | ||
if (!isMobile) return; | ||
hideWidget(); | ||
return () => showWidget(); |
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.
Is this cleanup function been called? Isn't this function firing when isMobile
is changed? If so, what's the case that this variables changes?
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.
yes its running everytime ismobile changed. 'whats the case that this variable changes', I dont understand what your trying to say, I did not write the code for isMobile. You wont have the desired behavior if i remove that cleanup. change the width of your browser to mobile size, close the chat, now make the screen wider. The button is gone (its not needed anymore), now how will you open the chat?
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.
nevermind I understand what you want me to do.
src/Me/Navigation/Navbar.tsx
Outdated
@@ -126,3 +156,6 @@ const Logout = styled(NavItemDecoration)` | |||
margin-bottom: 10px; | |||
} | |||
`; | |||
function setIsTawkShowing(arg0: boolean): void { |
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.
Remove?
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.
ive removed this
src/utils/tawk.ts
Outdated
}; | ||
|
||
function init() { | ||
if (process.env.NODE_ENV === 'development' || isSsr()) { | ||
return; | ||
// return; |
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.
Let's keep the return.
It's rare we change things in the chat area and it's annoying to work on the project and keep getting the chat sounds and effects.
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.
ive done that
src/utils/tawk.ts
Outdated
window.Tawk_API.showWidget(); | ||
} | ||
|
||
//Example |
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.
Remove?
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.
remove the "//example" ok ive done that
where did you get it? Can it be used in terms of copyrights? I copied it from the dom. I dont know about copyrights your instructions said "use their image for the show / hide button" |
Improve tawk button position #815
"Tawk button is overlapping the logout button in the backoffice on mobile
Currently tawk's button is in the most top layer using z-index !important.
Seems like there is no way to configure this. The only way is to hide it and use tawk API to show / hide the chat."
by "improve position" I dont think you want css postion fix, you want:
tawk should behave like it normally behaves from a ui perspective as it does on the live site, i.e. the chat bubble will show and clicking on it opens the chat, this will happen everywhere except on the /me route (backoffice) && less than 800px where there will be no bubble
UI-wise, we can use their image for the show / hide button
there will be no bubble at all in backoffice on mobile, instead need to add button when you click it show dialog clicking second time hide dialog.
@moshfeu please accept pull request