-
Notifications
You must be signed in to change notification settings - Fork 279
refactor: use modular popup for iframe popups #450
Conversation
this allows to set the background color in the popup, which is otherweise impossible. this might change a few people's background in regular iframe tiles, if they have kept the default background color. (which I doubt, first because it's quite grey, second because the page in the iframe probably has its own background color...)
Can we set background to white by default? Transparent is possibly not the best choice as then many pages (those that don't define background color) will be transparent. Browsers use white background by default so matching that might be good. |
Hmm... I am wondering now, why that is not an issue with my page that became gray. I'll investigate and switch to white in the process. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we set background to white by default?
Hmm... I am wondering now, why that is not an issue with my page that became gray. I'll investigate and switch to white in the process.
Because there are various aspects to it. It depends on the theme and also on whether we are talking about the iframe tile or iframe popup.
Neither is really great as the tile became transparent and the popup background is black (or close) so the text is unreadable.
This is tested on http://elg.no page
Did so. I was offset by the fact that my test iframe had an (Which was actually on purpose, looking at my commit message 1fc790e introducing that change. But I agree that white is a better default. And a user can always add a style to |
I also styled the whole popup. It will allow to define, e.g., One thing that came back to my mind now is, that
It should not be a big deal, I believe, because the user can always implement |
Heads up: I might not be able to finish reviewing it this weekend as I'll be away. |
Fine. That means more time for my family. :-) |
Thanks! |
This is another refactor to make maximum use of the modular popup with the following changes
BREAKING CHANGE: This changes some behavior:
transparentwhite, because otherwise, the background of the iframe would be awkwardly gray.iframeClasses
are applied topopup-container
. Notpopup
(including the blurred-out area around the popup). This seemed more intuitive. But I think, it could be changed to less intuitivepopup
to not break existing setups. (And possibly allow for styling the blurred-out area.)iframeStyles
are applied to the iframe tile. Not to thepopup-container
. This is mainly due to the way, the popup is structured. There simply is a facility to pass styles to a tile. But there is not a facility to apply styles directly to the popup. For the user, this means that styles (e.g. borders) do not include the popup title anymore.Fixes to these changes:
fixcustom.css
styles to reflect new structureiframeClasses
together withcustom.css
and forget aboutiframeStyles
In summary, I would favor these changes over implementing the old structure and believe that they are not of a very big impact to the user. Especially the
styles
options should (in my opinion) be disfavoured in comparison to usingclasses
options. This will make it possible for us to supply even more classes for easy customization.