ENH: layout changes for compact complex screens + row positioner widget #563

klauer · 2023-08-05T00:07:03Z

Description

Add 'single line positioner' variant
Tweaked the logic for "should we show a 'detailed tree' at each device layer", now taking into account whether or not the tree is nested (i.e., is it a top-level device?)
Tweaked some other layout things while attempting to squeeze in the space a bit more
Other things probably too
(TODO) Devices snuck their way in here but those need to be removed. Solely for testing purposes.
"Notes" feature needs to be re-added for positioners

Tweaks to the layout of the row positioner widget by @ZLLentz :

Shorter text in positioner row alarm label
Status container always visible
Dynamic showing/hiding of status info
Infinite arrangement/sizing/size policy nitpicking
Drop the concept of the whole widget getting a colored border during/after moves
No alarm borders inside the widget
Add some more accurate placeholder templates for twincat states things
Remove typhos stylesheet for the error pydm label widget

TODO

Zach will modify layout
Later: decide as to if there should be a new row category for templates? Aspect ratio buckets for templates?
Get feedback from ECS, then scientists, ...
Remove device UI files from this repo and move them to pcdsdevices prior to merging

Motivation and Context

Request was "always show the embedded screen!" (#532)
That unfortunately does not render well if you take it literally. I'm of the opinion that it shouldn't be taken literally and we should explore options like this one.

Oh, turns out there was an actual issue for this: closes #543
I must have read it and forgotten about it. The single row display for motors was what we used by default back at NSLS-II with CSS, so this was something I always wanted to see in typhos and only recently found the inspiration for.

How Has This Been Tested?

Minimally, with at2l0 and multi motor example

import ophyd
from ophyd import Component, Device, EpicsMotor, EpicsSignal
from qtpy import QtCore, QtWidgets

from typhos.suite import TyphosSuite

app = QtWidgets.QApplication([])

class MyMotors(ophyd.Device):
    m1 = Component(EpicsMotor, "AT2L0:XTES:MMS:01")
    m2 = Component(EpicsMotor, "AT2L0:XTES:MMS:02")
    m3 = Component(EpicsMotor, "AT2L0:XTES:MMS:03")
    m4 = Component(EpicsMotor, "AT2L0:XTES:MMS:04")
    m5 = Component(EpicsMotor, "AT2L0:XTES:MMS:05")
    m6 = Component(EpicsMotor, "AT2L0:XTES:MMS:06")
    m7 = Component(EpicsMotor, "AT2L0:XTES:MMS:07")
    m8 = Component(EpicsMotor, "AT2L0:XTES:MMS:08")
    m9 = Component(EpicsMotor, "AT2L0:XTES:MMS:09")
    m9 = Component(EpicsMotor, "AT2L0:XTES:MMS:09")
    m10 = Component(EpicsMotor, "AT2L0:XTES:MMS:10")
    m11 = Component(EpicsMotor, "AT2L0:XTES:MMS:11")
    m12 = Component(EpicsMotor, "AT2L0:XTES:MMS:12")
    m13 = Component(EpicsMotor, "AT2L0:XTES:MMS:13")
    m14 = Component(EpicsMotor, "AT2L0:XTES:MMS:14")
    m15 = Component(EpicsMotor, "AT2L0:XTES:MMS:15")
    m16 = Component(EpicsMotor, "AT2L0:XTES:MMS:16")
    m17 = Component(EpicsMotor, "AT2L0:XTES:MMS:17")
    m18 = Component(EpicsMotor, "AT2L0:XTES:MMS:18")
    m19 = Component(EpicsMotor, "AT2L0:XTES:MMS:19")


device = MyMotors(name="device")
suite = TyphosSuite.from_device(device)
suite.show()
app.exec_()

Where Has This Been Documented?

Screenshots (if appropriate):

Latest screenshot with first-pass adjustments from Zach and now the new notes:

Previous screenshot(s) from WIP

Click here for typhos master screenshot for comparison (ew!):

at2l0:

It'd be nice to regain a bit more vertical space - but in any case, we still have drastically improved over the master branch version in that regard

ZLLentz · 2023-08-07T16:37:05Z

The concept here is solid

There's some friction with the templates in https://github.com/pcdshub/pcdsdevices/tree/master/pcdsdevices/ui which might need a simultaneous PR, for feeling-good-about-packaging reasons we've kept typhos to only having templates that match up with ophyd built-ins

I think the layouts themselves are infinitely nitpick-able, I'd like to sit down together and nitpick maximally and agree on a final layout. It's worth comparing notes with https://github.com/pcdshub/ecs-pydm-ui/blob/master/widgets/positioner_row.ui

klauer · 2023-08-08T14:51:49Z

There's some friction with the templates in https://github.com/pcdshub/pcdsdevices/tree/master/pcdsdevices/ui which might need a simultaneous PR, for feeling-good-about-packaging reasons we've kept typhos to only having templates that match up with ophyd built-ins

Yeah, 100% agreed
Buried in the PR description is "Devices snuck their way in here but those need to be removed. Solely for testing purposes."

I think the layouts themselves are infinitely nitpick-able, I'd like to sit down together and nitpick maximally and agree on a final layout.

Definitely - let's set up a meeting so we're all happy with this and then we can talk about how to move forward

It's worth comparing notes with https://github.com/pcdshub/ecs-pydm-ui/blob/master/widgets/positioner_row.ui

Ah, I wasn't aware of this one. Maybe that should be taken as a starting point instead of what I have here.

klauer · 2023-08-08T21:20:27Z

I think I'll need to pull out the screenshot feature into a separate PR so we can effectively diff this row positioner vs the standard one

klauer · 2023-08-09T19:12:35Z

(Rebased on #566 ; may do that again post-review/merge)

Clear_error blocking should not stop typhos from exiting

klauer · 2023-08-15T20:53:50Z

(The tests are broken because this PR temporarily changes internal templates. Those will be removed before this is marked as ready for review)

In the meantime, I'm passing this over to @ZLLentz as he has some tweaks to do on the row positioner widget

ZLLentz

I only saw one edge case bug, I'm pretty happy with how these look
Pushed a small commit to address Robert's screenshot

ZLLentz · 2023-08-29T23:36:49Z

docs/source/upcoming_release_notes/563-row_positioner.rst

This is the biggest pre-rel I've ever seen

ZLLentz · 2023-08-29T23:41:10Z

typhos/positioner.py

+        panel = TyphosSignalPanel()
+        panel.omitNames = self.get_names_to_omit()
+        panel.sortBy = SignalOrder.byName
+        panel.add_device(self.device)


missing is here is filtering the new signal panel by the kinds specified in the menu above

interactively right now, if you deselect e.g. config or add omitted it only applies to existing signal panels

Good catch - that needs addressing

Edit: hmm, this should be finding the signal panels: https://github.com/klauer/typhos/blob/45c6e5cc546674a1285886ddc7d7c54526304753/typhos/display.py#L373
Needs further investigating - perhaps in a follow-up PR?

In more words: the filters work, but they only apply to positioner rows that have already been expanded at least once, because otherwise the signal panel has never been created

I'll copy my original comment now that I fully understand - "good catch - that needs addressing!" 😁

I think the linked code will have to be modified to tell the positioner widget (when it's ready) how to create the signal panel with the appropriate kind settings

fwiw I think this is a small thing and could be ignored and doubled-back on later
the main drawback of the status quo is that all the omitted signals appear for the first expansion

tangkong · 2023-08-31T16:26:54Z

This LGTM. The additional items mentioned make sense as follow up issues / PRs. I'm ready with my green stamp whenever.

Did we want to solicit more scientist/PoC feedback first? I'm relatively sure this will be well received, but maybe I'm out of touch

klauer · 2023-08-31T16:36:49Z

This LGTM. The additional items mentioned make sense as follow up issues / PRs. I'm ready with my green stamp whenever.

Zach has noted he's positive on this, so:

I'll do a couple fix-ups that are really required (removing device templates from this PR)
Then we merge
Then I'll open a pcdsdevices PR with changes for using these new templates
And maybe I'll address some of the concerns above as well in separate typhos PRs (for Zach/your approval)

Did we want to solicit more scientist/PoC feedback first? I'm relatively sure this will be well received, but maybe I'm out of touch

I think the best kind of feedback we can get is from them actually using the screens rather than nitpicking a screenshot. At least that's the quickest path to making everyone happi(er) with less effort - or so I think

klauer · 2023-09-01T17:44:24Z

OK, I made a bunch of issues. Feel free to add more context to those or rephrase them as desired @ZLLentz @tangkong

I'm going to merge this and update the other in-progress PRs

One step closer!

ZLLentz · 2023-09-06T00:26:49Z

@klauer should I proceed to update the pcdsdevices templates tomorrow?

vespos · 2023-09-10T16:42:34Z

Did we test the layout on controls rooms machines to make sure it's not broken there?

tangkong · 2023-09-11T15:28:20Z

I've tested on control room machines remotely (via ssh), but not in person. I assume the latter is what you are concerned about?

ZLLentz · 2023-09-12T17:44:14Z

I did test this in TMO, anticipating issues, and made further changes to the template based on that testing.
It's worth testing again anyway.

vespos · 2023-09-12T19:04:08Z

Yes exactly, I've had issues with my pydm screens where they did not look as intended on the CR machines.
I'll test it at xpp as well, using the code snippet in the PR description

ZLLentz · 2023-09-12T19:44:39Z

Yeah, it's pretty annoying dealing with these sorts of scaling issues. We can either define fonts in terms of point-sizes, in which case they'll be readable on high-dpi displays at the risk of clipping, or we can define them in terms of pixel-sizes, in which case they'll often be hard to read on high-dpi displays but won't clip. A comprehensive solution isn't built into qt and so we find ourselves testing and retesting these (or not testing and then having bad displays)

klauer changed the title ~~WIP/POC: positioner which uses horizontal space - a "one line positioner"~~ WIP/POC: positioner which uses horizontal space - a "positioner row" widget Aug 8, 2023

klauer mentioned this pull request Aug 9, 2023

ENH: add --screenshot to the typhos CLI #566

Merged

klauer force-pushed the enh_oneline_positioner branch from d241c20 to 9205e2a Compare August 9, 2023 19:12

klauer added 7 commits August 14, 2023 10:34

WIP/POC: positioner which uses horizontal space

9533338

CLN: remove prototype collapsible widget

7549719

MNT: rename 'one line positioner' -> 'positioner row'

47ea46a

REF: always suggest tree for top-level screens

76c8c74

MNT: move device name label to top

3ca4ad5

MNT: add back title in positioner

ac43191

TMP: patch over issue 565 temporarily

4f102df

klauer force-pushed the enh_oneline_positioner branch from 9205e2a to 4f102df Compare August 14, 2023 17:34

klauer added 7 commits August 15, 2023 11:17

FIX: patch over issue 565

a00bba4

WIP: tweaks for positioner row after discussion

a75ac10

ENH: 'expand panel' option for positioner row

06c3966

MNT: add type hints + fix expand button

5a42a84

FIX: clear error daemon thread

ccb8f40

Clear_error blocking should not stop typhos from exiting

MNT: more alignment fixes

c0709d4

MNT: clear out status label text

b82f263

klauer changed the title ~~WIP/POC: positioner which uses horizontal space - a "positioner row" widget~~ ENH: layout changes for compact complex screens + row positioner widget Aug 15, 2023

MNT: placeholder for size policy fixing

b0ac7cf

klauer and others added 5 commits August 15, 2023 13:54

Merge branch 'master' into enh_oneline_positioner

df39abf

MNT: basic layout nitpicking

751c08a

WIP: try bigger readback and moving up the name

2c7dc7f

ENH: layout adjustments for resizing and error handling

9b9aaf4

ENH: increase bounding boxes and fill out the placeholder templates

e919aae

klauer and others added 3 commits August 28, 2023 17:14

MNT: hide user_setpoint in row widget

3190483

FIX: size hint depending on mode

c7322a2

FIX: more reasonable minimum width for row widget

45c6e5c

ZLLentz reviewed Aug 29, 2023

View reviewed changes

tangkong approved these changes Aug 31, 2023

View reviewed changes

MNT: remove development device templates

4b616c5

klauer mentioned this pull request Aug 31, 2023

REF: include row positioner templates for new typhos pcdshub/pcdsdevices#1159

Merged

7 tasks

klauer and others added 4 commits August 31, 2023 14:04

MNT: remove unintentional change

aaf937d

Merge branch 'master' into enh_oneline_positioner

8da1096

FIX: widgets may be gc'd before screenshot; ignore them

f7ba0db

FIX: cherry-pick fixup

60763fc

klauer merged commit 249468c into pcdshub:master Sep 1, 2023
7 of 9 checks passed

klauer deleted the enh_oneline_positioner branch September 1, 2023 17:44

tangkong mentioned this pull request Sep 5, 2023

Feedback: Switch default view to 'embedded' #532

Closed

ZLLentz mentioned this pull request Sep 7, 2023

REF: button bar rework - template switch menu + Jira reporting button #572

Merged

klauer mentioned this pull request Sep 8, 2023

Switching to templates without title bars #580

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ENH: layout changes for compact complex screens + row positioner widget #563

ENH: layout changes for compact complex screens + row positioner widget #563

klauer commented Aug 5, 2023 •

edited

Loading

ZLLentz commented Aug 7, 2023 •

edited

Loading

klauer commented Aug 8, 2023

klauer commented Aug 8, 2023

klauer commented Aug 9, 2023

klauer commented Aug 15, 2023

ZLLentz left a comment

ZLLentz Aug 29, 2023

ZLLentz Aug 29, 2023

klauer Aug 29, 2023 •

edited

Loading

ZLLentz Aug 30, 2023

klauer Aug 30, 2023

ZLLentz Aug 31, 2023

klauer Sep 1, 2023

tangkong commented Aug 31, 2023

klauer commented Aug 31, 2023

klauer commented Sep 1, 2023

ZLLentz commented Sep 6, 2023

vespos commented Sep 10, 2023

tangkong commented Sep 11, 2023

ZLLentz commented Sep 12, 2023

vespos commented Sep 12, 2023 •

edited

Loading

ZLLentz commented Sep 12, 2023 •

edited

Loading

ENH: layout changes for compact complex screens + row positioner widget #563

ENH: layout changes for compact complex screens + row positioner widget #563

Conversation

klauer commented Aug 5, 2023 • edited Loading

Description

Motivation and Context

How Has This Been Tested?

Where Has This Been Documented?

Screenshots (if appropriate):

ZLLentz commented Aug 7, 2023 • edited Loading

klauer commented Aug 8, 2023

klauer commented Aug 8, 2023

klauer commented Aug 9, 2023

klauer commented Aug 15, 2023

ZLLentz left a comment

Choose a reason for hiding this comment

ZLLentz Aug 29, 2023

Choose a reason for hiding this comment

ZLLentz Aug 29, 2023

Choose a reason for hiding this comment

klauer Aug 29, 2023 • edited Loading

Choose a reason for hiding this comment

ZLLentz Aug 30, 2023

Choose a reason for hiding this comment

klauer Aug 30, 2023

Choose a reason for hiding this comment

ZLLentz Aug 31, 2023

Choose a reason for hiding this comment

klauer Sep 1, 2023

Choose a reason for hiding this comment

tangkong commented Aug 31, 2023

klauer commented Aug 31, 2023

klauer commented Sep 1, 2023

ZLLentz commented Sep 6, 2023

vespos commented Sep 10, 2023

tangkong commented Sep 11, 2023

ZLLentz commented Sep 12, 2023

vespos commented Sep 12, 2023 • edited Loading

ZLLentz commented Sep 12, 2023 • edited Loading

klauer commented Aug 5, 2023 •

edited

Loading

ZLLentz commented Aug 7, 2023 •

edited

Loading

klauer Aug 29, 2023 •

edited

Loading

vespos commented Sep 12, 2023 •

edited

Loading

ZLLentz commented Sep 12, 2023 •

edited

Loading