Add window position to SettingsManager (JSON + CLI) #136
Conversation
assets/settings.json
Outdated
| @@ -16,6 +16,7 @@ | |||
| "fullscreen": true, | |||
| "borderless": false, | |||
| "size": {"x": 1280, "y": 720}, | |||
| //"pos": {"x": -1, "y": -1}, //Window appears centered if no position specified | |||
There was a problem hiding this comment.
Does this actually parse successfully? I thought Cinder's JSON parser was pretty strict. Either way, I'd be a bit torn about leaving a comment in the JSON itself, since that's technically not supported in pure JSON. What do you think @adielfernandez ?
There was a problem hiding this comment.
Yup, it compiles and runs just fine. But I agree, it's not great practice, but I wanted to add it to start this conversation of how to let users know while keeping the default at a centered window.
There was a problem hiding this comment.
Yeah. I wonder if we should just set it to 0/0 by default (meaning the window would be top/left) and add a note in the readme to remove it if you want it to auto-center.
There was a problem hiding this comment.
That works, but I'd recommend something like 50, 50 so that the toolbar up top is on screen and people can drag the window where they need.
There was a problem hiding this comment.
Ah I see... Maybe we should just leave it out then? I feel like in most cases it should just work. Even if we wanted to position a window at -1/-1, that should happen automatically if the window is centered and +2px wide/tall.
There was a problem hiding this comment.
Sounds good to me 👍
SettingsManager defaults to window centered on screen if no position is set. Wanted the default setting for the block to be a centered window but felt like the position setting line should still be included, so the line is commented out in the settings.json template. Let me know if you prefer another default @benjaminbojko