-
Notifications
You must be signed in to change notification settings - Fork 78
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
interactive: added label to display selected queries to user (5/7) #1773
Conversation
…played in the output)
Co-authored-by: Sasha Lopoukhine <[email protected]>
…ld tests merged into one larger one due to shared global variable + asynch tests.
…unction description + editing comment line length in code to fit column size.
….e make a new slate). test added as well to test this.
Codecov ReportAll modified and coverable lines are covered by tests ✅
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It feels a little off for the borders to be unaligned above and below, could you please make them line up?
I keep getting confused with all of these PRs and how they are ordered. Could you add an index to the PR title? Something like |
I originally did that and then Sasha told me it wasn't helpful. Will do this now though. |
Classic, supervisors giving conflicting advice 😉 |
I've done it now - I think its helpful 😆 |
@superlopuh Addressed all your comments. But GitHub has blocked on merging, can you unblock me please? |
Please merge main and I'll take a look |
main merged :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looking good, just want to avoid the same class variable situation with the new setter
xdsl/interactive/app.py
Outdated
@@ -76,6 +79,13 @@ def __init__(self): | |||
self.passes_selection_list = SelectionList(id="passes_selection_list") | |||
super().__init__() | |||
|
|||
query_label = Label("", id="selected_passes_label") | |||
"""Display's user selected passes""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"""Display's user selected passes""" | |
"""Display selected passes.""" |
xdsl/interactive/app.py
Outdated
query_label = Label("", id="selected_passes_label") | ||
"""Display's user selected passes""" | ||
|
||
passes_selection_list: SelectionList[type[ModulePass]] = SelectionList( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please move to init
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also please move variable declarations above init
xdsl/interactive/app.py
Outdated
@@ -76,6 +79,13 @@ def __init__(self): | |||
self.passes_selection_list = SelectionList(id="passes_selection_list") | |||
super().__init__() | |||
|
|||
query_label = Label("", id="selected_passes_label") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
query_label = Label("", id="selected_passes_label") | |
selected_passes_label = Label("", id="selected_passes_label") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all comments addressed
added label to display selected queries to user.