Popup Disappearing problem solved#3
Conversation
VladimirMikulic
left a comment
VladimirMikulic
left a comment
There was a problem hiding this comment.
Please resolve minor issues.
|
We can also add a scroll event. By this when we scroll the page the popup will disappear, which in turn makes the user experience better. |
|
Yes @NitinBhasneria I think anything that means focusing outside of the pop-up should make it disappear. |
|
@NitinBhasneria maybe it's not a bad idea but it should trigger on a "bigger" scroll. Triggering popup removal on a slight scroll (a few pixels) which I sometimes accidentally do (others probably do as well) on my trackpad would be annoying to the users. |
|
Ok thanks for the idea @jywarren @VladimirMikulic |
NitinBhasneria
force-pushed
the
popup-nitin
branch
from
778921c to
7210e8d
Compare
|
@VladimirMikulic @jywarren Have a look now. |
|
This looks really good! but @VladimirMikulic were you saying you think it may need to be on a scroll of more than X pixels, rather than a repeated scroll? Just to clarify before we get ready to merge this! And, once we decide the exact behavior we want, @NitinBhasneria can you rename the title and first line of description above to clearly lay out the change in behavior? This will help us produce good documentation and track all the changes that go into each release. Thank you!!! 🙌 |
src/prompts/prompt.js
Outdated
| // Disappearing the popup when scrolled. | ||
| window.onscroll= function() { | ||
| scrollSize++; | ||
| if (scrollSize>10) { |
There was a problem hiding this comment.
This should be formatted correctly.
src/prompts/prompt.js
Outdated
| } | ||
|
|
||
| function rootClick (e) { | ||
| var classlist = e.target.classList.value; |
There was a problem hiding this comment.
classlist is too generic. Which element is this that we are referencing? Also, follow the camelCase naming convention.
| dom.input.focus(); | ||
| } | ||
|
|
||
| function rootClick (e) { |
There was a problem hiding this comment.
The function needs to be documented.
|
@jywarren Nitin covered the case I mentioned. 👍 |
|
OK, great! Thanks, everyone! @NitinBhasneria once this is done could you open a PR to bump the version number on |
created #6 |
4 participants
Fixes: publiclab/PublicLab.Editor#407
Presently the toolbar popup is not disappearing. After this fix, if we click anywhere else the popup will disappear.
In this PR, a new crossvent function is added, this function will be called when we click anywhere except the popup box.