status-im / status-fiddle

Online UI editor for status-react
10 stars 9 forks source link

popular screen sizes added. FIX #9 #17

Closed ghost closed 6 years ago

ghost commented 6 years ago

Fixes #9

status: ready

flexsurfer commented 6 years ago

thanks @poiga works good, two moments 1) is it possible to use droplist instead buttons and positioning above screen? image 2)looks like for android devices width and height reversed

flexsurfer commented 6 years ago

@poiga doesn't work, looks like you need to use select onchange

ghost commented 6 years ago

@flexsurfer Should be good now! Please check! Strange - it worked on Firefox even before but now I test in chrome as well!

flexsurfer commented 6 years ago

@poiga and what about platform support in the editor, so we can show differences in views for ios and android https://github.com/status-im/status-fiddle/blob/master/src/cljs/status_im/utils/platform.cljs ?

flexsurfer commented 6 years ago

hey @audriu i can see your changes. about platform support let's add just current-os, and change it depends on current device, and in platform read this variable, and add this ns to compile, i don't think we need this ns how it works in status-react because we will need two versions of fiddle for ios and android then, what do you think?

ghost commented 6 years ago

@flexsurfer I think I got you. I will make changes and I let's see...

ghost commented 6 years ago

@flexsurfer I think that instead 2 versions of fiddle we should build a preprocessor for the android1 andios` tags?

flexsurfer commented 6 years ago

@audriu don't think i got your idea, let's implement simple version, so when you change device in listbox we should not only change size but also recompile if device os was changed, and add platform ns alias, in editor we should be able to use

[react/test {style {if (platform/ios?} 2 3}

platform/platform should return "ios" or "android" string platform/ios? false/true platform/android? false/true

ghost commented 6 years ago

@flexsurfer please check my last commit! Is it similar to your idea?

ghost commented 6 years ago

@flexsurfer and please reload the default template! I added (if (platform/ios?) :white :red)

flexsurfer commented 6 years ago

@audriu yes, but this will not work, because user ns knows nothing about re-frame we need to use atom and reset! it manually. also i made some changes please rebase

ghost commented 6 years ago

@flexsurfer I think it is done now!

flexsurfer commented 6 years ago

hey @audriu this will not work too, just wondering do you make changes without running code? :) so i think this will be simplest solution

"(ns cljs.platform)
                                  (def platform \"" os "\")
                                  (def android? " (= os "android") ")"
                                 "(def ios? " (= os "ios") ")"

i'll merge your PR and make changes, this will be faster, thanks

ghost commented 6 years ago

@flexsurfer it did work! seriously it did work with (if (platform/ios?) :white :red). May be you wanted some different syntax, but it did work just like that.

ghost commented 6 years ago

@flexsurfer please try with snippet:

[react/text {:style {:color(if (platform/ios?) :green :red)}}"Test"]

this also works when you switch the device.

flexsurfer commented 6 years ago

@audriu sorry if this sounded rude or incredulously, i just wondering what is your developing process :) the problem was it should be def not defn and i have code (if platform/ios? :white :red) without parentheses , so i found simplest way how to do it, and decided to use it, your solution is good, but for defn , but i need def thank you for your effort