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

[O2B-1327] User interface for setting GAQ detectors #1690

Merged
merged 209 commits into from
Aug 9, 2024

Conversation

xsalonx
Copy link
Collaborator

@xsalonx xsalonx commented Jul 31, 2024

I have a JIRA ticket

  • branch and/or PR name(s) include(s) JIRA ID
  • issue has "Fix version" assigned
  • issue "Status" is set to "In review"
  • PR labels are selected

Notable changes for users:

  • Added form for choosing GAQ detectors

Notable changes for developers:

  • Added 'gaq' role to (dev) UserRoleSelectionModel

Changes made to the database:

  • NA

@xsalonx xsalonx changed the title [O2B-1327] Set GAQ detectors [O2B-1327] User interface for setting GAQ detectors Aug 6, 2024
Copy link
Member

@graduta graduta left a comment

Choose a reason for hiding this comment

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

There are some improvements that could and should be done on the UI:

  • given that you already load the run data, thus detectors part of that run, should the modal not include only those detectors? Otherwise we always display a DB error message. It will be easier for user to know which eligible detectors there are to select from;
  • please have a look at display flex and justify-content so that checkboxes are aligned vertically with the label.

@xsalonx xsalonx requested a review from graduta August 8, 2024 13:10
graduta
graduta previously approved these changes Aug 9, 2024
@xsalonx xsalonx requested a review from graduta August 9, 2024 13:32
@xsalonx xsalonx merged commit 0823c24 into main Aug 9, 2024
21 of 23 checks passed
@xsalonx xsalonx deleted the xsalonx/gaq/O2B-1327/set-detectors branch August 9, 2024 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

2 participants