Make buttons work on Work screen and various other UI things #55
Conversation
|
Ugh. I'm sorry my linter made a mess of the javascript diff for this PR :( |
|
User testing issue.
|
|
@NeilHanlon Thanks for the pr. The UI is getting pretty busy, i'm thinking of moving it to a proper vue.js project to break it up into components. Once you address my comments, I can merge this and then make a GH action to do proper UI builds so we don't end up with a 5000 line html and 8000 line js 😅 |
Yep, makes sense to me. I'm happy to rebase my stuff on top if you want to reorganize first. I want to squash a couple bugs in this before it goes in, anyways (Mostly the UX things River mentioned) |
|
@builder555 im not seeing your comments on my side. I think you may have to click the "publish review" button in the review screen |
ui/settings.html
Outdated
| <div class="row-item"> | ||
| <div> | ||
| <i class="fa-solid fa-gauge-high"></i> {{liveData.Watts}} W | ||
| <span class="peak" style="font-size: 0.8em; display: block; margin-top: -0.5em;">peak</span> |
There was a problem hiding this comment.
this text makes the number higher than the other two, perhaps consider moving the text or moving this value elsewhere?
another thought, is this required if a graph is something that'll be added in the near future?
There was a problem hiding this comment.
Yes even with graph, having box to show peak watts is good bc you can't watch graph at every second while also soldering . This is why on joric graph I asked for specific display of whatever peak is in whole session. Also we can't record the graph now either so peak box is the only reference we can see after running for a while.
Even then I think it's good Yes needed for ease to find it even if there is graph. and it puts a feature in this that is not even in the Professional station software.
There was a problem hiding this comment.
Latest commit should bring these all in line, as well as remove the 'peak' word.
There was a problem hiding this comment.
Will leave this open re: if peak wattage is needed. IMO, it's nice to have, but not 'needed' per se. That said, I do like having it.
| <button class="button change plus"> <i class="fas fa-plus"></i> </button> | ||
| <div style="font-size:0.3em; margin-top:-1.8em; margin-bottom:0.8em">SET: {{settings.SetTemperature.value}}°C</div> | ||
| <div class="buttons are-large"> | ||
| <button class="button change minus" @click="setTemperature(-1, $event)" @touchstart="setTemperature(-1, $event)" @mousedown="setTemperature(-1, $event)" @touchleave="setTemperature(0, $event)" @mouseleave="setTemperature(0, $event)" @mouseup="setTemperature(0, $event)" @touchend="setTemperature(0, $event)"> <i class="fas fa-minus"></i> </button> |
There was a problem hiding this comment.
while holding down the key is good replica of the device, it's usually not advisable on the web apps for accessibility reasons. if the user needs to quickly change to a much larger or smaller value, we have presets for that.
There was a problem hiding this comment.
-
do you think the +/- buttons should just be short press (which simulates the increment in the short temp change setting?
-
and remove the long press funtion which is fixed now and currently simulates whatever the user has for increment in Long temp change setting. so if 10 is there, long press does 10.
I was able to do this fine on Windows and Android. on Iphone while I was able to do it, I must note that iphone when you "long hold" on the (+) or (-) enlarges/magnifies the screen. I'm not sure if that is something in my settings I need to turn off - I didn't get chance to investigate. I think this has to do with zoom settings perhaps. Do you mean "long" press might cause issues with some phone settings for some people since people have all different accessibility settings so it's better to only have the short press option? -
builder, will you be implementing the ability to be able to type directly on the "Set °C" number so that it can be typed there directly instead of just using the +/- buttons? Kneel said he was going to leave that to you to implement. as it looks like you already did it for the blue boxes so it would just be going onto the "Work View" to be able to type onto that spot to change to 307 for example. I think this would be nice because right now during testing I have been scrolling down to the "soldering settings" just to be able to type a number directly into the box to change it.
There was a problem hiding this comment.
as River mentioned, the latest commit has the long/short press matching that of the pinecil better and works on desktop+mobile.
from a UI perspective, I can see how it's not very accessible. Perhaps that is something we can iterate on later?
There was a problem hiding this comment.
from a UI perspective, I can see how it's not very accessible.
so far it's amazingly working right now after the fixes done, on both iphone and Android for long button presses to increment long change temp on +/-
so I think keep them until something comes up?
that's my mistake, i added them but forgot to click "submit review" |
* Make plus/minus buttons control set temperature as per Short/Long press parameters. Tapping the buttons once change the set point by TempChangeShortStep. Holding them changes the set point by TempChangeLongStep * Work Screen changed to add preset buttons which immediately set the iron to the specified temperature. Holding down this button will prompt to confirm setting a new preset value using the current SetPoint. * Presets are stored in localstorage in the browser * Peak wattage is displayed alongside other live data * display proper temperature based on setting Signed-off-by: Neil Hanlon <neil@shrug.pw> Signed-off-by: Neil Hanlon <neil@rockylinux.org>
Work screen improvements and functionality implmementation
Closes #32
press parameters. Tapping the buttons once change the set point by
TempChangeShortStep. Holding them changes the set point by
TempChangeLongStep
iron to the specified temperature. Holding down this button will
prompt to confirm setting a new preset value using the current
SetPoint.