ManiacalLabs / SimPixel

WebGL Pixel Art Visualizer
http://simpixel.io
GNU General Public License v3.0
9 stars 7 forks source link

Two redundant controls - "Toggle" and "Fullscreen". #24

Closed rec closed 7 years ago

rec commented 7 years ago

Both of them seem to do the same thing.

mwcz commented 7 years ago

That's one button with the text "Toggle Fullscreen"

screenshot from 2017-03-27 11-31-37

rec commented 7 years ago

Ah, so the bug should be something like: "Toggle Fullscreen button is split over two lines in MacOS" (tested on Chrome and Safari): see screenshot:

screen shot 2017-03-27 at 17 38 04

adammhaile commented 7 years ago

WTF? No idea why... you have firefox on that machine? How's it look in there. Since Safari and Chrome both use webkit... just want to rule that out. It's not like that line is too long... other items are longer. What's your current zoom level?

@mwcz Offending line is here: https://github.com/ManiacalLabs/SimPixel/blob/master/src/js/view.js#L170 I can't see anything I did wrong... can you?

@rec If you inspect the element does anything pop out to you? I don't currently have a mac that reliably works to really test this...

rec commented 7 years ago

Same issue on FF!

I also tried on another Mac, which is unfortunately running exactly the same version of MacOS - that being 10.9.5. Same issue.

The element looks completely vanilla to me.

I have one clue - I tried replacing the text in that field in the browser. Basically, you can put in Toggle Fullscree and it all stays on one line. It's only when you add that final n that it wraps.

I experimented, and the wrap length appears to be at or near where the checkbox for Show Dark LEDs start.

adammhaile commented 7 years ago

Weird... I could probably increase the width of the dat.gui pane but I don't want to... solution: Change it to just say "Fullscreen". I was trying to be pedantic, but I think people will understand that the toggle is implied.

rec commented 7 years ago

Great, low-work solution. :-)

On Mon, Mar 27, 2017 at 8:05 PM, Adam Haile notifications@github.com wrote:

Weird... I could probably increase the width of the dat.gui pane but I don't want to... solution: Change it to just say "Fullscreen". I was trying to be pedantic, but I think people will understand that the toggle is implied.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ManiacalLabs/SimPixel/issues/24#issuecomment-289535382, or mute the thread https://github.com/notifications/unsubscribe-auth/AAPdstrxBu_gUWhSl-xBTiumf3Tok2qwks5rp_pigaJpZM4MqSR- .

-- /t

https://tom.ritchford.com https://tom.ritchford.com https://tom.swirly.com https://tom.swirly.com

adammhaile commented 7 years ago

Side note: I expect this has to do with the 96 vs 72 DPI rendering difference between Mac OS and Windows/Linux. When I wrote cross platform Mac/Windows software I ended up writing this weird calculation into the templating engine such that it would find the default system UI font size and then apply a scaling to it if it was Mac so that the Windows and Mac version would actually have the same text size... Because just saying 14pt font for both ended up in different size rendered text on each system.

rec commented 7 years ago

All my crossplatform stuff used JUCE which handles that behind the scenes. I'm glad I never had to do that. :-D

On Mon, Mar 27, 2017 at 8:08 PM, Adam Haile notifications@github.com wrote:

Side note: I expect this has to do with the 96 vs 72 DPI rendering difference between Mac OS and Windows/Linux. When I wrote cross platform Mac/Windows software I ended up writing this weird calculation into the templating engine such that it would find the default system UI font size and then apply a scaling to it if it was Mac so that the Windows and Mac version would actually have the same text size... Because just saying 14pt font for both ended up in different size rendered text on each system.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ManiacalLabs/SimPixel/issues/24#issuecomment-289536309, or mute the thread https://github.com/notifications/unsubscribe-auth/AAPdstIsjXiCAFGxqb83Pz1TO312n0EWks5rp_spgaJpZM4MqSR- .

-- /t

https://tom.ritchford.com https://tom.ritchford.com https://tom.swirly.com https://tom.swirly.com

adammhaile commented 7 years ago

Fix committed and uploaded to beta.simpixel.io

rec commented 7 years ago

Works!

On Mon, Mar 27, 2017 at 8:13 PM, Adam Haile notifications@github.com wrote:

Fix committed and uploaded to beta.simpixel.io

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ManiacalLabs/SimPixel/issues/24#issuecomment-289537565, or mute the thread https://github.com/notifications/unsubscribe-auth/AAPdsntETTK_ae1nMEPG_isgFm_uK75Lks5rp_w1gaJpZM4MqSR- .

-- /t

https://tom.ritchford.com https://tom.ritchford.com https://tom.swirly.com https://tom.swirly.com

adammhaile commented 7 years ago

:+1:

mwcz commented 7 years ago

That's so bizarre! We should report this as a bug against datgui.

adammhaile commented 7 years ago

I think what is most weird is that it shows up as sort of a full new button...

On Mon, Mar 27, 2017 at 2:26 PM, Michael Clayton notifications@github.com wrote:

That's so bizarre! We should report this as a bug against datgui.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/ManiacalLabs/SimPixel/issues/24#issuecomment-289541555, or mute the thread https://github.com/notifications/unsubscribe-auth/AA6a6oAGxNCD3iZj-6O_yPBcDMp__jlkks5rp_9JgaJpZM4MqSR- .

mwcz commented 7 years ago

Yeah, that's definitely the weirdest part. The extra "button" looks similar to a datgui folder, like these:

screenshot from 2017-03-27 15-57-29