Pop-up menu opens outside visible area

[braykov]

Have a wiki UI adjusted to 80% screen for the tiddlers and leaving 20% screen for the sidebar. Have the down-arrow shown on the sidebar. Click the down-arrow icon ("More Actions") in the sidebar.

Result: the menu opens positioned in a way that its upper left corner is just bellow the down-arrow. This way most of the menu is not seen (placed outside the browser visible area.

The sidebar gets a horizontal scrollbar at the bottom and one can scroll manually to be able to see the full popup menu area. Expected: There is enough space in the browser and the menu should be displayed a bit to the left so it is all visible and no scrolling is required.

[Spangenhelm]

@braykov +1 same things with long names in sidebar, creates the scrollbar -> annoying.

But what about when the menu is hidden ? would it not "break" the layout ?

[Jermolene]

same things with long names in sidebar, creates the scrollbar -> annoying.

If you're suggesting introducing word wrap for the sidebar then I think that that is a separate issue. The popup positioning is a straightforward fix, but I think the word wrapping thing is likely to be controversial.

[Jermolene]

Sorry closed in error

[Spangenhelm]

If you're suggesting introducing word wrap for the sidebar

I had nothing particular in mind but anything that can help us reduce scrolling (vertically or horizontally) would be appreciated, if word wrap could help then yes it should be considered.

..the word wrapping thing is likely to be controversial.

You are right ;) Shall we open a new issue for it anyway?

[danielo515]

This is also an issue for me, specially on mobile. I am working on a theme for mobile, but things like popups are hard to style because they use inline styles that can not be overriden. I wrote a small modification of the reveal widget to avoid this, but there is no automatical change while on mobile, you have to use a different reveal widget, so similar problem.

[Jermolene]

Hi @danielo515

I am working on a theme for mobile, but things like popups are hard to style because they use inline styles that can not be overriden

Which styles do you want to change? Obviously, the position properties "left" and "top" have to be specified inline because they are dynamically generated, but I guess we could use a class to assign the "z-index" and "position:absolute;". The trouble is that that would be a backwards incompatible change.

[danielo515]

Hello @Jermolene

Which styles do you want to change? Obviously, the position properties "left" and "top" have to be specified inline because they are dynamically generated,

Yes, exactly those. Maybe instead of inline styles we can use dynamic classes. The position values are stored in a temporary tiddler so maybe it's possible to generate a CSS rule instead. Anyway those values does not make any sense on mobile, at least the left one. As I said I wrote a widget that does not apply the offset specified as parameter, but I can not distinguish if I'm on mobile or not. Maybe that is what we need for the current widget.

[Arlen22]

In other words, if you are on mobile or perhaps if the sidebar is on top, a popup should just be placed in the center of the screen taking up an appropriate width. Maybe full screen width or content width, whichever is smaller. We are already used to that on mobile, just not desktop.

I did something similar with the popup mechanism on TWC. I can't remember how I did it, but I remember that I made so that the popup would extend up to the top of the page once it hit the bottom. I also had it so that the scrollbars were on the divs and the page was window height. We could do the same thing for the width, where it would make sure that it's not going past the window when it gets placed. This would be another way to do it. On Dec 17, 2014 2:52 PM, "danielo515" notifications@github.com wrote:

Hello @Jermolene https://github.com/Jermolene

Which styles do you want to change? Obviously, the position properties "left" and "top" have to be specified inline because they are dynamically generated,

Yes, exactly those. Maybe instead of inline styles we can use dynamic classes. The position values are stored in a temporary tiddler so maybe it's possible to generate a CSS rule instead. Anyway those values does not make any sense on mobile, at least the left one. As I said I wrote a widget that does not apply the offset specified as parameter, but I can not distinguish if I'm on mobile or not. Maybe that is what we need for the current widget.

— Reply to this email directly or view it on GitHub https://github.com/Jermolene/TiddlyWiki5/issues/763#issuecomment-67382357 .

[Jermolene]

Maybe instead of inline styles we can use dynamic classes.

Generating dynamic CSS class definitions would be quite a big change, and require the invention of some new mechanisms. And I don't think it would fix the problem: to make the styles unique we'd either have to qualify the classnames, or use scoped stylesheets. In either case it would be hard for a plugin to override.

Anyway those values does not make any sense on mobile, at least the left one

To me, the ideal behaviour of popup sizing and positioning would be that popups are prevented from expanding beyond the width of the screen, and their positions are adjusted to keep them on the screen. I For wide popups, that would have the effect of moving their left position to zero. Would that behaviour achieve what you want?

[danielo515]

I For wide popups, that would have the effect of moving their left position to zero. Would that behaviour achieve what you want?

That is what my modification of the popup widget does. I'ts very simple:

/*\
title: $:/danielo515/modules/widgets/revealNoOffset.js
type: application/javascript
module-type: widget

Reveal widget without left offset 

\*/

(function(){

/*jslint node: true, browser: true */
/*global $tw: false */
"use strict";

var reveal = require("$:/core/modules/widgets/reveal.js").reveal;

var RevealNoOffset = function(parseTreeNode,options) {
    this.initialise(parseTreeNode,options);
};

RevealNoOffset .prototype = new reveal();
RevealNoOffset .prototype.parentexecute = RevealNoOffset.prototype.execute;

RevealNoOffset.prototype.execute = function (){
this.parentexecute();
this.noOffset =  this.getAttribute("offset","left");
};

RevealNoOffset .prototype.readPopupState = function(state) {
    var popupLocationRegExp = /^\((-?[0-9\.E]+),(-?[0-9\.E]+),(-?[0-9\.E]+),(-?[0-9\.E]+)\)$/,
        match = popupLocationRegExp.exec(state);
    // Check if the state matches the location regexp
    if(match) {
        // If so, we're open
        this.isOpen = true;
        // Get the location
        this.popup = {
            left: this.noOffset === "left" ? 0 : parseFloat(match[1]) ,
            top: this.noOffset === "top" ? 0 : parseFloat(match[2]),
            width: parseFloat(match[3]),
            height: parseFloat(match[4])
        };
    } else {
        // If not, we're closed
        this.isOpen = false;
    }
};

exports["reveal-no-offset"] = RevealNoOffset ;

})();

[Jermolene]

Hi @danielo515 thank you. It doesn't seem like a particularly generic solution to the problem of popups opening off-screen, though. Under what circumstances would the user set the "offset" attribute?

[danielo515]

Hello @Jermolene

That is the problem of my solution: is very particular an focused on a particular use-case. The ideal solution would be that the widget itself can calculate the screen resolution and know if it is out of screen. Maybe we can use a macro that can return nothing if the screen is wide enough or left if it is on mobile?

[braykov]

Until this is fixed, this is what I did as a workaround:

Control Panel -> Appearance -> Toolbars -> Page Toolbar: I rearranged the items, putting the More actions button to the top of the list

[pmario]

@Jermolene ... The dropdown in the OP does not exist anymore and there are several other issues that also talk about popup positions which are much "newer" in date. .. So IMO this one should be closed.

[Arlen22]

@Jermolene ... The dropdown in the OP does not exist anymore and there are several other issues that also talk about popup positions which are much "newer" in date. .. So IMO this one should be closed.

Closing. I'm fine with that. Oops, sorry, I thought I was the OP. Nevermind!

[Jermolene]

Thanks @pmario – that dropdown is still present, it's just by default not configured to be visible as a page control button. As far as I can tell the bug described in the OP still exists.

[yaisog]

Hi @Jermolene, I have a similar problem with popups that extend too far down and are clipped when the container overflow is set to hidden or clip. For lateral overflow I was able to create some CSS using calc() to correct a position too far to the right (a relative positioned child is moved as necessary). For vertical overflow this does not work, as parent height is not available in CSS for calculations. My idea was to extend the RevealWidget with a clamp parameter that corrects left and top values as needed, since within this widget I will have access to the child popup width and height for calculations. I will try to whip up a demo if I get it to work.

Looking at the code, I had another question about the width and height coordinates in the popup state tiddler. These are used in the coordinate calculation for popups, but are always 0 for me. In which situations are these set to non-zero values?

Yaisog

[yaisog]

Adding the below code to the RevealWidget.prototype.positionPopup function before the line if (this.popup.absolute) { worked well for me in limited testing:

if(this.clampToParent) {
    var offsetParentDomNode = domNode.offsetParent,
        parentWidth = offsetParentDomNode.offsetWidth,
        parentHeight = offsetParentDomNode.offsetHeight,
        right = left + domNode.offsetWidth,
        bottom = top + domNode.offsetHeight;
    if(domNode.offsetWidth <= parentWidth && right > parentWidth) {
        left = parentWidth - domNode.offsetWidth;
    }
    if(domNode.offsetHeight <= parentHeight && bottom > parentHeight) {
        top = parentHeight - domNode.offsetHeight;
    }
    // clamping on left and top sides is taken care of by positionAllowNegative
}
// TODO: clampToParent handling for absolute positions

The execute function got the following additional line:

this.clampToParent = this.getAttribute("clamp","no") === "yes";

requiring explicit activation of the feature.

Like the comment says I did not do any testing for absolute positions. I'm not sure at the moment if any special handling is even needed there. That's the reason I've not made a PR, yet.

Yaisog

[pmario]

I think you should create a PR, so an automatic preview will be created, where we can see a difference. I did try it and I do have no idea how to use it. It does do nothing for me.

[yaisog]

Alright, I made a PR #7898 with preview. A demo tiddler to play around with opens at the top of the preview.