search

Oitzu / pimatic-iframe

GNU General Public License v2.0
2 stars 5 forks source link

iframe percentage width patch #14

Closed bstrebel closed 7 years ago

bstrebel commented 7 years ago

Allow px or % in iframe width specification. Simple and straight forward. Works for me and a value of 99.5% gives an iframe that fits very well in the pimatic layout. Should be reviewed by someone more experienced in frontend development.

Oitzu commented 7 years ago

@bstrebel is this a proposal to fix #7 ?

bstrebel commented 7 years ago

@Oitzu The requirements of #7 are partly addressed. My PR allows percentage values in the width specification only. I think it doesn't make much sense for the mobile-frontend to specify the height of the iframe as an relative value. I've done only minor changes to your code and it works well on my devices.

A configuration of config

leads to

landscape

and

portrait

I'm happy with this, but as already mentioned, I'm not experienced in frontend development and someone should review the code changes.

Would be great if the patch is ok for you and can be committed to the master. Existing configurations should be not affected.

Oitzu commented 7 years ago

@bstrebel thanks for the informations it's very appreciated. The best persons to review the change are probably @mwittig or @thost96 .

bstrebel commented 7 years ago

@mwittig , @Oitzu : Any chance to get the PR merged? Thanks, Bernd

Oitzu commented 7 years ago

@bstrebel i would love to, but i'm also not really up to date on this topic. I will take a look at the weekend and try to evaluate it myself.

mwittig commented 7 years ago

I am happy to look into this. I also wanted to review again another pull request we had earlier and possibly combine both to a working solution.

acefed commented 7 years ago

I wonder if there are plans to merge this PR, is this still under review?

mwittig commented 7 years ago

Actually, I had reviewed and tested the the PR a while ago, but forgot to report here. I think the PR is good to go. @Oitzu could you please merge it or grant me for co-maintainership in case you're too busy. Thanks

Oitzu commented 7 years ago

@mwittig I have limited access to my computer till the weekend. Granted you collaborator to not delay this. ;)

acefed commented 7 years ago

Awesome, thanks a lot!

acefed commented 7 years ago

Thanks for the original commit and superswift merge today!

I updated to pimatic-iframe version 0.3.0 via the webinterface, and the layout scales perfectly now as seen on the screenshots above. Huzzah! Tested with Firefox 55.0.2.

bstrebel commented 7 years ago

Thanks to all for reanimating and merging the patch!