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

Fixing border-radius on mobile & hiding close button on desktop #194

Closed
wants to merge 1 commit into from

Conversation

toi500
Copy link

@toi500 toi500 commented Jun 13, 2024

  1. Fixed a minor CSS misconfiguration with the left and right top borders on mobile
image

--

  1. Hid the close button on desktop and made it visible only when needed on mobile (full screen)
0002.mp4
  1. No conflicts with the full-screen chat embedded
0001png
  1. Live demo
    pop-up: https://flowise-test03.surge.sh/
    full-screen: https://flowise-test03.surge.sh/index-full

@amansoni7477030
Copy link
Contributor

@toi500 I think it's not a good idea to hide the close button on desktop because some users don't know how to actually close the chatbot if they don't see a close button. A good example of this is available here: https://www.chatbase.co/. In this example, the chatbot can be closed by clicking on the icon as well as the close button.

@toi500
Copy link
Author

toi500 commented Jun 13, 2024

@amansoni7477030 I get your point but I have never encountered a user who asked here in Flowise or anywhere else how to close the chatbot after opening it by clicking the chatbot button. It is quite intuitive and the way other main players do it.

The reasons for the change are as follows:

  1. Preserve the minimalist and default view of the chatbot.
  2. Limit the chances of accidentally deleting your history by clicking the wrong button. Now, to use properly a RAG, the chat history is quite important for users.

@amansoni7477030
Copy link
Contributor

amansoni7477030 commented Jun 13, 2024

@toi500 check :
https://www.chatbase.co/
https://www.botsonic.com/
https://devrev.ai/

This are some major chatbot service providers they all provoide a functionality for closing bot from both clicking on the icon as well as the close button.

@toi500
Copy link
Author

toi500 commented Jun 13, 2024

@amansoni7477030 my friend, I already told I got your point. I could also provide a list of other chatbot players that do not use it, but what is the point?

But since you've already brought up some of them, Botonic (https://www.botsonic.com/) and DevRev (https://devrev.ai/) don't have a chat history deletion function, so there's no way for users to accidentally delete their history by closing it.

And this is the main reason I made this PR:

  • Limit the chances of accidentally deleting your history by clicking the wrong button. Now, to use properly a RAG, the chat history is quite important for users.
image image

If @HenryHengZJ (or any other member of the team) thinks this change is unncesary, I have no problem to close this PR, or bad feelings about it either. I just think is a good chage and wanted to share it.

@amansoni7477030
Copy link
Contributor

amansoni7477030 commented Jun 13, 2024

@toi500 I have no issue with this PR; I was just sharing my opinion on it. I think we should wait and let @HenryHengZJ decide what he wants for this change.

@toi500
Copy link
Author

toi500 commented Jun 13, 2024

@toi500 I have no issue with this PR; I was just sharing my opinion on it. I think we should wait and let @HenryHengZJ decide what he wants for this change.

We can at least agree we both want the best possible embbed chatbot in flowise 😉

@amansoni7477030
Copy link
Contributor

@toi500 I have no issue with this PR; I was just sharing my opinion on it. I think we should wait and let @HenryHengZJ decide what he wants for this change.

We can at least agree we both want the best possible embbed chatbot in flowise 😉

Yes, thats what i want

@amansoni7477030
Copy link
Contributor

Hi @toi500 @HenryHengZJ, what do you think about the feature with the three dots containing extra options?
Check below screenshot:

Screenshot from 2024-06-14 12-40-35

}

@media (min-width: 640px) {

.py-2.pr-3.absolute.top-0.right-\[-8px\].m-\[6px\] {
Copy link
Contributor

Choose a reason for hiding this comment

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

i think if we want to hide it, it should be done on the component level, not css. beacuse .py-2.pr-3.absolute.top-0.right-\[-8px\].m-\[6px\] { <- this will not work once we change the style of that button

Copy link
Author

Choose a reason for hiding this comment

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

I agree. This level of specificity only works well when you are cooking your own web.js for a specific project.

@amansoni7477030
Copy link
Contributor

Hii @HenryHengZJ can you please also review #192 this pull request

@HenryHengZJ
Copy link
Contributor

HenryHengZJ commented Jun 14, 2024

also thank you guys so much for the contribution! I personally dont have preference. I dont think it matters much to end user as well. If I have to pick one, for consistency, I suppose we can leave it there.

}

.chatbot-button {
padding-right: 12px !important;
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need this? do you have a screenshot of the differences?

Copy link
Author

Choose a reason for hiding this comment

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

This was a change I had to intruduce after seeing that the items (clear and close) does not share a flex container. So now the clear botton has a fixed padding-right: 48px, resulting: #183 (comment) if you elimine the close botton.

image

@HenryHengZJ
Copy link
Contributor

the close button is lil bit off in my opinion, not coming from this PR:
image

@toi500 toi500 closed this Jun 14, 2024
@toi500
Copy link
Author

toi500 commented Jun 14, 2024

@HenryHengZJ after your feedback, I am going to close this PR as it is unnecessary. Me (and any other educated user in the matter) can always make these sorts of changes in our own custom web.js anyway.

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.

None yet

3 participants