Skip to content

Indicate to the user that a stash save is in progress #1702

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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
Closed

Indicate to the user that a stash save is in progress #1702

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c9e6ff7
Add and use AsyncStash implementation
May 28, 2023
adc43d1
Display stash message and disable input on AsyncStash
May 28, 2023
8102e2b
Update CHANGELOG.md
May 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion src/components/stashmsg.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use super::{
use crate::{
keys::{key_match, SharedKeyConfig},
queue::{InternalEvent, NeedsUpdate, Queue},
strings,
strings::{self},
tabs::StashingOptions,
ui::style::SharedTheme,
};
Expand Down Expand Up @@ -65,6 +65,12 @@ impl Component for StashMsgComponent {

if let Event::Key(e) = ev {
if key_match(e, self.key_config.keys.enter) {
self.input.disable();
self.input.set_title(
strings::stash_popup_stashing(
&self.key_config,
),
);
self.git_stash.stash_save(
if self.input.get_text().is_empty() {
None
Expand Down Expand Up @@ -138,6 +144,7 @@ impl StashMsgComponent {
///
pub fn update_git(&mut self, ev: AsyncGitNotification) {
if self.is_visible() && ev == AsyncGitNotification::Stash {
self.input.enable();
self.input.clear();
self.hide();

Expand Down
15 changes: 14 additions & 1 deletion src/components/textinput.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,13 @@ pub enum InputType {
}

/// primarily a subcomponet for user input of text (used in `CommitComponent`)
#[allow(clippy::struct_excessive_bools)]
pub struct TextInputComponent {
title: String,
default_msg: String,
msg: String,
visible: bool,
disabled: bool,
show_char_count: bool,
theme: SharedTheme,
key_config: SharedKeyConfig,
Expand All @@ -57,6 +59,7 @@ impl TextInputComponent {
Self {
msg: String::new(),
visible: false,
disabled: false,
theme,
key_config,
show_char_count,
Expand Down Expand Up @@ -222,6 +225,16 @@ impl TextInputComponent {
self.default_msg = v;
}

///
pub fn disable(&mut self) {
self.disabled = true;
}

///
pub fn enable(&mut self) {
self.disabled = false;
}

fn get_draw_text(&self) -> Text {
let style = self.theme.text(true, false);

Expand Down Expand Up @@ -419,7 +432,7 @@ impl Component for TextInputComponent {
}

fn event(&mut self, ev: &Event) -> Result<EventState> {
if self.visible {
if self.visible && !self.disabled {
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is pretty invasive. why do we need that? the text input for the stashing can just close IMO and does not need to learn a new state of disabled. what is the rational?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@nrempel

Copy link
Author

Choose a reason for hiding this comment

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

Without disabled, it's possible to type in the field while the operation is pending.

I suppose we could close the model and continue the operation in the background. In this case, is there a pattern for handling stashing errors if they occur?

Copy link
Collaborator

Choose a reason for hiding this comment

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

yeah that's what I think we should do, what we do in other cases already. just close the text input popup regularly and then the new popup will block any other input and visualise that the stashing is in progress

Copy link
Collaborator

Choose a reason for hiding this comment

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

for error handling use try_or_popup and find other examples for it

Copy link
Author

Choose a reason for hiding this comment

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

Thanks, I'll make that change soon 👍🏻

if let Event::Key(e) = ev {
if key_match(e, self.key_config.keys.exit_popup) {
self.hide();
Expand Down
3 changes: 3 additions & 0 deletions src/strings.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,9 @@ pub fn stash_popup_title(_key_config: &SharedKeyConfig) -> String {
pub fn stash_popup_msg(_key_config: &SharedKeyConfig) -> String {
"type name (optional)".to_string()
}
pub fn stash_popup_stashing(_key_config: &SharedKeyConfig) -> String {
"Stashing...".to_string()
}
pub fn confirm_title_reset() -> String {
"Reset".to_string()
}
Expand Down