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

Multiple pop-ups at once when tool buttons are clicked. #750

Closed
NARUDESIGNS opened this issue Oct 14, 2021 · 42 comments · Fixed by #809
Closed

Multiple pop-ups at once when tool buttons are clicked. #750

NARUDESIGNS opened this issue Oct 14, 2021 · 42 comments · Fixed by #809
Labels

Comments

@NARUDESIGNS
Copy link
Collaborator

Screenshot 2021-10-13 at 12 30 05 AM

I clicked on several tool buttons such (e.g table, link etc...) and all corresponding popups occurs. However, they stay on screen until you click again on the icon that triggered the pop-up.

I expect a pop-up to close when I do one of the following:

  • click the esc button,
  • click on the page,
  • click on another pop-up (to close previously opened pop-up).

This issue can be replicated on the publiclab editor by clicking the tool buttons as you see in the image above.

@welcome
Copy link

welcome bot commented Oct 14, 2021

Thanks for opening your first issue! This space is protected by our Code of Conduct - and we're here to help.
Please follow the issue template to help us help you 👍🎉😄
If you have screenshots or a gif to share demonstrating the issue, that's really helpful! 📸
Do join our Gitter channel for some brainstorming discussions.

@NARUDESIGNS
Copy link
Collaborator Author

@TildaDares I'm willing to work on this, please.

@TildaDares
Copy link
Member

Good catch @NARUDESIGNS! Go ahead🎉

@NARUDESIGNS
Copy link
Collaborator Author

Thank you @TildaDares I will attempt to work on it and revert.

@RaviAnand111
Copy link
Contributor

hey @NARUDESIGNS are you working on it, if you are busy, I can work on it ?

@TildaDares
Copy link
Member

Hi @NARUDESIGNS, can @RaviAnand111 work on this?

@NARUDESIGNS
Copy link
Collaborator Author

Sure, @RaviAnand111 can work on it. I'm currently working on completing the conversion of all test files from jasmine tests to jest tests.
cc @TildaDares

@RaviAnand111
Copy link
Contributor

Hey guys, I found out that, to make popups to disappear on click on the page or another pop-up is to add
$("...").attr('data-trigger', 'focus');, so I have added it in table and others but I am not able to find module or code for attachments, insert link, insert image where can I find those modules code. ?
@TildaDares @NARUDESIGNS @jywarren
Thanks

@TildaDares
Copy link
Member

@RaviAnand111 You should be able to make those changes in this file

wysiwyg.stylePrompt = function() {
$(".wk-prompt button, span.wk-prompt-browse").addClass("btn btn-outline-secondary");
$(".wk-prompt input")
.addClass("input form-control")
.css("margin-bottom", "5px");
};
$(
".wk-commands button.woofmark-command-attachment, .wk-commands button.woofmark-command-image"
).click(wysiwyg.stylePrompt);

@RaviAnand111
Copy link
Contributor

Hey @TildaDares , where can I find .wk-prompt .wk-command classes in html, I can't find it, it is just being called by jquery. and it is somehow working.
@NARUDESIGNS

@RaviAnand111
Copy link
Contributor

image

Hey @TildaDares @NARUDESIGNS, as in the image, click has been crossed in vs code and on hovering it is saying it has been deprecated so is this function working or not?

@RaviAnand111
Copy link
Contributor

2022-01-08.20-27-04.mp4

Hi everyone, I wanted to know your views on it ? Am I missing something here ?

@RaviAnand111
Copy link
Contributor

why are we using older bootstrap version and functions ?

@NARUDESIGNS
Copy link
Collaborator Author

NARUDESIGNS commented Jan 8, 2022

Hey @TildaDares @NARUDESIGNS, as in the image, click has been crossed in vs code and on hovering it is saying it has been deprecated so is this function working or not?

@RaviAnand111 I think it still works, I've seen some other events crossed in vscode as well.

@NARUDESIGNS
Copy link
Collaborator Author

NARUDESIGNS commented Jan 8, 2022

I don't know much how popovers work yet but this was the approach I wanted to take:
I think there is something like $('.element').popover('hide') and so I wanted to do a check if the esc button is clicked, or if an element that isn't the popped over modal is clicked then based on that, I'd trigger the $('.element').popover('hide') method to hide it.
@RaviAnand111

@RaviAnand111
Copy link
Contributor

Ok, Thanks, @NARUDESIGNS I will try doing that.

@NARUDESIGNS
Copy link
Collaborator Author

NARUDESIGNS commented Jan 10, 2022

I tried something that worked but it will require a little more code. Here's the snippet I tried:

 $('.woofmark-command-table').popover({html: true});

+  let popoverIsOpen = false;
  $('.wk-commands .woofmark-command-table').click(function() {
+    popoverIsOpen = !popoverIsOpen;
    $('.ple-table-size').click(function(e) {
      wysiwyg.runCommand(function(chunks, mode) {
        var table = createTable(
            +Number($('.ple-table-popover #tableCols').text()),
            +Number($('.ple-table-popover #tableRows').text())
        );

        if (mode === 'markdown') chunks.before += table;
        else {
          chunks.before += _module.wysiwyg.parseMarkdown(table);
          setTimeout(_module.afterParse, 0); // do this asynchronously so it applies Boostrap table styling
        }

        $('.woofmark-command-table').popover('toggle');
      });
    });

+    $(':not(".woofmark-command-table")').click((e) => {
+      if (popoverIsOpen && !e.target.classList.contains("woofmark-command-table")) {
+        // console.log("popover is opened and should now close");
+        $('.woofmark-command-table').click();
+      }
+    });
  });

The lines with the "+" sign are the lines I added. And the file is PublicLab.RichTextModule.Table.js from line 79.
What I did was basically add a popoverIsOpen boolean so that when the popover is triggered, this boolean is set to true.
the part where you see $(':not(".woofmark-command-table")').click... is basically saying, if any element is clicked other than the table button (which usually triggers the popover), call the click() method on the table button which will then close the popover and set popoverIsOpen to false.

This works but with some issues, if you click on any elements in the popover, it will also close the popover since the rule is to close it when any element other than the table button is clicked. This will require more code, but the way I plan to tackle this one is to have all the classes of elements in the popover in an array and do a check against them in the if block.

@RaviAnand111
Copy link
Contributor

This is great, and the idea with array is cool too, I was getting confused in this part of how to shut down pop over when something else is clicked.
@NARUDESIGNS

@RaviAnand111
Copy link
Contributor

I have also find a way , with least no of code lines.

@NARUDESIGNS
Copy link
Collaborator Author

I have also find a way , with least no of code lines.

Awesome, please share. @RaviAnand111

@RaviAnand111
Copy link
Contributor

How do I share the code like you did ? @NARUDESIGNS

@NARUDESIGNS
Copy link
Collaborator Author

NARUDESIGNS commented Jan 12, 2022

` ```javaScript

// code in here

``` `

Please use 3 backticks, not 4
@RaviAnand111

@RaviAnand111
Copy link
Contributor

See the last line with + in this code

     $('.woofmark-command-table').attr('data-content', builder);
     $('.woofmark-command-table').attr('data-container', 'body');
     $('.woofmark-command-table').attr('data-placement', 'top');
  + $('.woofmark-command-table').attr('data-trigger', 'focus');

By just adding this, button is doing everything we want, execpt one thing,
popup is disappearing on clicking everything but the popup button, to make that happen I am trying to write code,

@NARUDESIGNS
Copy link
Collaborator Author

OK cool. But I have a question, when you click on the popover itself or any of it's child elements like the - and + buttons, does it disappear?
@RaviAnand111

@RaviAnand111
Copy link
Contributor

No, id does not, @NARUDESIGNS

@NARUDESIGNS
Copy link
Collaborator Author

OK, cool!

@RaviAnand111
Copy link
Contributor

I have a doubt, let me check

@RaviAnand111
Copy link
Contributor

actually, it is disappearing on clicking its child elements, sorry about that, I didn't check it earlier.

@RaviAnand111
Copy link
Contributor

so, now I'd have to write code for popup button and child elements except add button also.

@RaviAnand111
Copy link
Contributor

What do you think ?

@NARUDESIGNS
Copy link
Collaborator Author

actually, it is disappearing on clicking its child elements, sorry about that, I didn't check it earlier.

Oh ok, this was the issue with my own approach as well. How do you intend to tackle this?

@RaviAnand111
Copy link
Contributor

I don't know yet, I have solved the problem with the popup button and now it can be used to disappear, now I just have to do the same with child buttons, I will try to do that, and tell you what I got.

@RaviAnand111
Copy link
Contributor

hey guys, I am so sorry, I gave my whole day to this problem and I still couldn't solve it, I am getting more and more confused.
@NARUDESIGNS @TildaDares

@NARUDESIGNS
Copy link
Collaborator Author

It's okay @RaviAnand111 we all feel like that sometimes. I'm currently confused with what actions to take for some of my task but we can always communicate to see how we can make progress 💪

@NARUDESIGNS
Copy link
Collaborator Author

So for now, you've been able to solve the part where a click on the page or on the table button can hide the popover.

But the problem is that when you click on elements in the popover, it also hides the popover right?

@RaviAnand111
Copy link
Contributor

sure, @NARUDESIGNS , thanks,

@RaviAnand111
Copy link
Contributor

yes that is the problem

@NARUDESIGNS
Copy link
Collaborator Author

See the last line with + in this code

     $('.woofmark-command-table').attr('data-content', builder);
     $('.woofmark-command-table').attr('data-container', 'body');
     $('.woofmark-command-table').attr('data-placement', 'top');
  + $('.woofmark-command-table').attr('data-trigger', 'focus');

Can you please tell me what file you added this line to?

@NARUDESIGNS
Copy link
Collaborator Author

I'd also work on my initial solution (though it's more code than yours) to see if I'm able to solve this problem you're having as I also have that problem with my initial solution.

@RaviAnand111
Copy link
Contributor

yeah sure, the file is src/modules/PublicLab.RichTextModule.Table.js,
and there is 1 more problem, on adding the code I said, the popover is not disappearing on clicking on the table button so that is also a problem to be solved,
overall the solution you gave is much better than the code I advised,
so tell me what should we do.

@NARUDESIGNS
Copy link
Collaborator Author

Ok, maybe what I can do is to try to fix it, make a PR and we can evaluate the changes together, and based on that, we can solve for other popovers. How about that?

@RaviAnand111
Copy link
Contributor

Ok, I am going to make a PR then

RaviAnand111 added a commit to RaviAnand111/PublicLab.Editor that referenced this issue Jan 18, 2022
RaviAnand111 added a commit to RaviAnand111/PublicLab.Editor that referenced this issue Jan 18, 2022
RaviAnand111 added a commit to RaviAnand111/PublicLab.Editor that referenced this issue Jan 21, 2022
RaviAnand111 added a commit to RaviAnand111/PublicLab.Editor that referenced this issue Jan 21, 2022
RaviAnand111 added a commit to RaviAnand111/PublicLab.Editor that referenced this issue Jan 26, 2022
RaviAnand111 added a commit to RaviAnand111/PublicLab.Editor that referenced this issue Jan 26, 2022
RaviAnand111 added a commit to RaviAnand111/PublicLab.Editor that referenced this issue Jan 26, 2022
RaviAnand111 added a commit to RaviAnand111/PublicLab.Editor that referenced this issue Jan 26, 2022
RaviAnand111 added a commit to RaviAnand111/PublicLab.Editor that referenced this issue Jan 26, 2022
jywarren pushed a commit that referenced this issue Jan 26, 2022
#809)

* Custom Insert popover hide on clicking on anything on the screen fixes #750

* Custom Insert popover hide on clicking on anything on the screen fixes #750

* Custom Insert popover hide on clicking on anything on the screen fixes #750

Co-authored-by: Tilda Udufo <mathildaudufo@gmail.com>
jywarren added a commit that referenced this issue Jan 27, 2022
#800)

* Table popup hide on click on page and everything except table button and its children

* Table popup hide on click on page and everything except table button and its children fixes #750

* Table popup hide on click on page and everything except table button and its children fixes #750

* Table popup hide on clicingk on page and everything except table button and its children fixes #750

* Table popup hide on clicingk on page and everything except table button and its children fixes #750

Co-authored-by: Jeffrey Warren <jeff@unterbahn.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment