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

Singleton mode doesn't work #2647

Closed
Zash opened this issue Sep 13, 2021 · 18 comments
Closed

Singleton mode doesn't work #2647

Zash opened this issue Sep 13, 2021 · 18 comments
Labels
bug UI User interface

Comments

@Zash
Copy link
Contributor

Zash commented Sep 13, 2021

Describe the bug
The Prosody web chat is using the embedded view mode

To Reproduce
Steps to reproduce the behavior:

  1. Configure with both singleton:true and view_mode:"embedded"
  2. ???
  3. Blank page

Roug minimal config along these lines:

converse.initialize({
"auto_join_rooms" : ["group@chat.example"],
"auto_login" : true,
"blacklisted_plugins" : [
	"converse-controlbox",
],
"bosh_service_url" : "/bosh",
"discover_connection_methods" : false,
"jid" : "anonymous.sasl.chat.example",
"singleton" : true,
"view_mode": "embedded"
});

Setting singleton:false (or commenting it out) makes it sorta work, but the single chat does not take up the entire viewport, leaving space roughly the size of the missing controlbox.

More complete example with HTML etc in the form of an attempt to upgrade chat.prosody.im to Converse.js 8.x

Expected behavior
Expected the Prosody web chat but with Converse.js 8.x goodies :)

Screenshots

Imagine a blank page.

Environment (please complete the following information):

  • Firefox 78 ESR (also tested on FF 91 earlier)
  • Converse.js v8.0.0

Additional context
Add any other context about the problem here.

@lunika
Copy link

lunika commented Sep 15, 2021

We have the same issue on Marsha.
The converse-root element exists in the DOM but we don't see it.
We also use the root configuration settings. It works but not like in version 7.

With the exact same config, you can see the difference between converse 8 and 7 :

Converse 7

Screenshot 2021-09-15 at 15-34-19 Screenshot

converse 8

Screenshot 2021-09-15 at 15-24-26 Screenshot

@rscmbbng
Copy link

Can confirm we have the same issue on a few webchats, all use the singleton option.

@jcbrand
Copy link
Member

jcbrand commented Sep 24, 2021

I believe this should be fixed now. Let me know if there are still issues.

@jcbrand
Copy link
Member

jcbrand commented Sep 24, 2021

BTW, this fix contains breaking changes, so will only be able to go out in Converse 9 (which probably won't take as long to release like Converse 8).

For now you'll have to make the builds yourself if you want to use the latest code.

@lunika
Copy link

lunika commented Oct 27, 2021

I have tested with the master branch and it works. Thanks.

@JohnXLivingston
Copy link
Contributor

BTW, this fix contains breaking changes, so will only be able to go out in Converse 9 (which probably won't take as long to release like Converse 8).

Can you please add in the v8.0.0 and/or v8.0.1 release notes that singleton mode is broken?
I just lost 2 hours to find why upgrading to v8.0.1 brokes my application. I finally saw in the changelog for the upcoming v9 that there is a fix for the singleton mode... (and yes, that was my bug: it works with singleton set to false).

@JohnXLivingston
Copy link
Contributor

Looking at your commit 84c6a00 , I see the following comment:

It's now necessary to add a `converse-root` element in the DOM where you
want Converse to render (previously it was any element with the id
`#conversejs`).

You have modified the demo/embedded.html to replace <div id="conversejs"></div> by <converse-root></converse-root>.

But the fullscreen.html file ( https://github.com/conversejs/converse.js/blob/master/fullscreen.html ) was not modified, and don't have the converse-root DOM element. It still uses <div id="conversejs-bg"></div>. It seems that this DOM id is still used here:

const bg = document.getElementById('conversejs-bg');

Is this correct?

What will be the correct way to have a singleton mode, in a fullscreen page? (for v9)

@jcbrand
Copy link
Member

jcbrand commented Nov 3, 2021

@JohnXLivingston The text you quoted is specifically when you want to control where Converse gets embedded into the DOM (which generally would be when you've set view_mode to embedded).

For a full page Converse (view_mode is fullscreen), it's not necessary to do this because you don't need to control where in the DOM Converse gets embedded and Converse will insert itself (as converse-root).

The <div id="conversejs-bg"></div> is something else (it has a different id attribute) and is just to show a Converse styled background when there is no chat open in fullscreen mode.

@JohnXLivingston
Copy link
Contributor

@JohnXLivingston The text you quoted is specifically when you want to control where Converse gets embedded into the DOM (which generally would be when you've set view_mode to embedded).

For a full page Converse (view_mode is fullscreen), it's not necessary to do this because you don't need to control where in the DOM Converse gets embedded and Converse will insert itself (as converse-root).

The <div id="conversejs-bg"></div> is something else (it has a different id attribute) and is just to show a Converse styled background when there is no chat open in fullscreen mode.

Thanks for your response!
Have you tested the singleton mode in fullscreen mode with your fix? (I can confirm that singleton mode is not working with v8.0.1).

@jcbrand
Copy link
Member

jcbrand commented Nov 4, 2021

I think I did, but it's been a while back.

It's fairly easy to build Converse from the latest source (at least on Linux or Mac).

Just check out the repo and then run make dist and then the files necessary are in the dist folder.

It would help if more people did that and reported whether their issues are fixed.

@JohnXLivingston
Copy link
Contributor

I'll test it next week, and tell you if it is working.
Thanks.

@JohnXLivingston
Copy link
Contributor

With the current version in the git repository, I can confirm that singleton mode is working for fullscreen mode.

But, there is a styling problem. I'll open a new issue.

@lunika
Copy link

lunika commented Nov 18, 2021

Hi @jcbrand this bug seems here again. Tested with commit e347621 and the doesn't appear at all like before.

@jcbrand
Copy link
Member

jcbrand commented Nov 19, 2021

@lunika: Looks fine to me. You need to rebuild the distribution files via make dist whenever you update your repo.

@lunika
Copy link

lunika commented Nov 19, 2021

Yes this is what I made. I built converse using make dist. The chat doesn't appear at all in the browser. I see it in the DOM and if the converse-chats element is changed by a div then it appears.

@jcbrand
Copy link
Member

jcbrand commented Nov 19, 2021

if the converse-chats element is changed by a div then it appears.

This sentence doesn't make sense to me. What do you mean?

@lunika
Copy link

lunika commented Nov 19, 2021

When I inspect the DOM with the developer console in my browser, in the code generated by converse, a <converse-chats /> element is added in the <converse-root /> element.

@jcbrand
Copy link
Member

jcbrand commented Nov 19, 2021

If a <converse-chats> element is being inserted, it means Converse is logged in and working, so is perhaps just a CSS issue.

Perhaps you just need to add the conversejs class to converse-root or to its parent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug UI User interface
Projects
None yet
Development

No branches or pull requests

6 participants