-
-
Notifications
You must be signed in to change notification settings - Fork 665
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
Introduce show_danger #4404
base: main
Are you sure you want to change the base?
Introduce show_danger #4404
Conversation
|
|
135f9b0
to
d8d236e
Compare
This is a unified interface for flow_warning_hi_prio, which was available only on Mercury before. [no changelog]
d8d236e
to
cba2e08
Compare
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.
A few points for discussion.
@@ -22,13 +22,13 @@ use super::super::{ | |||
}; | |||
|
|||
#[derive(Copy, Clone, PartialEq, Eq)] | |||
pub enum WarningHiPrio { | |||
pub enum Danger { |
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.
I's slightly prefer naming this ShowDanger
and also the file should be show_danger.rs
.
For consistency.
def show_danger( | ||
br_name: str, | ||
content: str, | ||
content_short: str | None = None, |
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.
I don't think the distinction between content
/content_short
is a good idea. I think we can keep the confirm_ethereum_unknown_contract_warning
function which calls the show_danger
with the appropriate string.
br_code: ButtonRequestType = ButtonRequestType.Warning, | ||
) -> Awaitable[None]: | ||
if content_short is None: | ||
content_short = content |
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.
apart from the previous comment, why showing content_short
on TT
?
This is a unified interface for
flow_warning_hi_prio
, which was available only on Mercury before.