-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Add prompt
function to the dialogue service.
#1350
Changes from 4 commits
aaaf298
51ce041
4eb777d
0e7d578
9715e2f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,8 @@ | ||
//! This module contains the implementation of a service | ||
//! to show alerts and confirm dialogs in a browser. | ||
//! | ||
//! If you call these methods repeatably browsers tend to disable these options to give users | ||
//! a better experience. | ||
|
||
use cfg_if::cfg_if; | ||
use cfg_match::cfg_match; | ||
|
@@ -41,4 +44,45 @@ impl DialogService { | |
feature = "web_sys" => utils::window().confirm_with_message(message).unwrap(), | ||
} | ||
} | ||
|
||
/// Prompts the user to input a message. In most browsers this will open an alert box with | ||
/// an input field where the user can input a message. | ||
/// | ||
/// It is possible to supply a default value which will be used if the user does not provide | ||
/// a value. | ||
/// | ||
/// [MDN Documentation](https://developer.mozilla.org/en-US/docs/Web/API/Window/prompt) | ||
/// | ||
/// This method will `panic!` if there is an error in the process of trying to carry out this | ||
/// operation. | ||
/// | ||
/// Note that this function is blocking; no other code can be run on the thread while | ||
/// the user inputs their message which means that the page will appear to have 'frozen' | ||
/// while the user types in their message. | ||
pub fn prompt( | ||
message: &str, | ||
#[cfg(feature = "web_sys")] default: Option<&str>, | ||
#[cfg(feature = "std_web")] _: Option<&str>, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need this argument on There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't, but it gets messy if functions have different signatures IMO. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We generate separate docs for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nope. I'll remove the argument on There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, I didn't think about the doc string! You can check out |
||
) -> Option<String> { | ||
cfg_if! { | ||
if #[cfg(feature="web_sys")] { | ||
if let Some(default) = default { | ||
utils::window() | ||
.prompt_with_message_and_default(message, default) | ||
.expect("Couldn't read input.") | ||
} | ||
else { | ||
utils::window() | ||
.prompt_with_message(message) | ||
.expect("Couldn't read input.") | ||
} | ||
} else if #[cfg(feature="std_web")] { | ||
let value: Value = js! { return prompt(@{message}); }; | ||
match value { | ||
Value::String(result) => Some(result), | ||
_ => 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.
Could you add a note about what this function returns. For example, when does it return
None
? I think we might be able to add a test for that case btwThere 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.
Ooh, that's a good idea. I want to look into developing a set of integration tests (ideally from the examples) as well so that we can test things which can only be tested in a browser.