Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MNT: increase timeout limits (limits on all form spinboxes), prevent aggressive navigation #259

Merged
merged 3 commits into from
Nov 6, 2024

Conversation

tangkong
Copy link
Contributor

@tangkong tangkong commented Oct 30, 2024

Description

  • increase the limit on spinboxes generated from MultiInputFormDialog, in response to a request that set-value-step timeouts not be limited to 99s (we have very slow motors)
  • navigate BACK to the parent page when creating a new check in SetValueStep
    • This negates the default behavior of StepPage, which selects new items when they are created.
    • Because SetValueStep initialized a row/new item without setting a PV, this resulted in extraneous navigation back to the parent page to set the PV, then back to the details to set the comparison information
    • Hacky but I'm too lazy to figure out a better way

Motivation and Context

Bug reports from @c-tsoi

How Has This Been Tested?

Interactively. Test suite still passes

Where Has This Been Documented?

This PR

Pre-merge checklist

  • Code works interactively
  • Code contains descriptive docstrings, including context and API
  • New/changed functions and methods are covered in the test suite where possible
  • Code has been checked for threading issues (no blocking tasks in GUI thread)
  • Test suite passes locally
  • Test suite passes on GitHub Actions
  • Ran docs/pre-release-notes.sh and created a pre-release documentation page
  • Pre-release docs include context, functional descriptions, and contributors as appropriate

@tangkong tangkong requested a review from ZLLentz October 30, 2024 16:55
@tangkong tangkong closed this Oct 30, 2024
@tangkong tangkong reopened this Oct 30, 2024
# A bit of a hack to prevent navigating to details prematurely
# normally an isinstance check would be good, but circular imports
try:
self.parent().parent().full_tree.select_by_data(self.data)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Briefly scrutinizing this: why is the parent's parent always the instance that owns the full tree?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The SetValueStep widget is inside a QWidget container, which is itself inside a StepPage(PageWidget) widget. only those PageWidget classes can see the full tree

@tangkong
Copy link
Contributor Author

tangkong commented Nov 6, 2024

I forgot about this. Merging now

@tangkong tangkong merged commit e09c76d into pcdshub:master Nov 6, 2024
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants