From 6953c06b465136eb987992ecbd62fa834dac45ef Mon Sep 17 00:00:00 2001 From: Christian Rocha Date: Thu, 10 Dec 2020 20:20:43 -0500 Subject: [PATCH] Improve markdown stashing and spinner logic in file listing --- ui/stash.go | 50 ++++++++++++++++++-------------------------------- ui/ui.go | 39 ++++++++++++++++++++++----------------- 2 files changed, 40 insertions(+), 49 deletions(-) diff --git a/ui/stash.go b/ui/stash.go index eb41753..2d8e26b 100644 --- a/ui/stash.go +++ b/ui/stash.go @@ -115,7 +115,6 @@ type stashModel struct { noteInput textinput.Model filterInput textinput.Model stashFullyLoaded bool // have we loaded all available stashed documents from the server? - loadingFromNetwork bool // are we currently loading something from the network? viewState stashViewState filterState filterState selectionState selectionState @@ -461,14 +460,13 @@ func newStashModel(general *general) stashModel { } m := stashModel{ - general: general, - spinner: sp, - noteInput: ni, - filterInput: si, - serverPage: 1, - loaded: NewDocTypeSet(), - loadingFromNetwork: true, - sections: s, + general: general, + spinner: sp, + noteInput: ni, + filterInput: si, + serverPage: 1, + loaded: NewDocTypeSet(), + sections: s, } return m @@ -487,7 +485,6 @@ func (m stashModel) update(msg tea.Msg) (stashModel, tea.Cmd) { m.err = msg.err m.loaded.Add(StashedDoc) // still done, albeit unsuccessfully m.stashFullyLoaded = true - m.loadingFromNetwork = false case newsLoadErrMsg: m.err = msg.err @@ -507,7 +504,6 @@ func (m stashModel) update(msg tea.Msg) (stashModel, tea.Cmd) { switch msg := msg.(type) { case gotStashMsg: m.loaded.Add(StashedDoc) - m.loadingFromNetwork = false docs = wrapMarkdowns(StashedDoc, msg) if len(msg) == 0 { @@ -551,13 +547,12 @@ func (m stashModel) update(msg tea.Msg) (stashModel, tea.Cmd) { return m, nil case spinner.TickMsg: - condition := !m.loadingDone() || - m.loadingFromNetwork || - m.viewState == stashStateLoadingDocument || - len(m.general.filesStashed) > 0 || - m.spinner.Visible() + loading := !m.loadingDone() + stashing := m.general.isStashing() + openingDocument := m.viewState == stashStateLoadingDocument + spinnerVisible := m.spinner.Visible() - if condition { + if loading || stashing || openingDocument || spinnerVisible { newSpinnerModel, cmd := m.spinner.Update(msg) m.spinner = newSpinnerModel cmds = append(cmds, cmd) @@ -572,15 +567,13 @@ func (m stashModel) update(msg tea.Msg) (stashModel, tea.Cmd) { } } + // Note: mechanical stuff related to stash success is handled in the parent + // update function. case stashSuccessMsg: - md := markdown(msg) - m.addMarkdowns(&md) - - if m.isFiltering() { - cmds = append(cmds, filterMarkdowns(m)) - } cmds = append(cmds, m.newStatusMessage("Stashed!")) + // Note: mechanical stuff related to stash failure is handled in the parent + // update function. case stashFailMsg: cmds = append(cmds, m.newStatusMessage("Couldn’t stash :(")) @@ -747,6 +740,7 @@ func (m *stashModel) handleDocumentBrowsing(msg tea.Msg) tea.Cmd { // Checks passed; perform the stash m.general.filesStashed[md.localID] = struct{}{} + m.general.filesStashing[md.localID] = struct{}{} cmds = append(cmds, stashDocument(m.general.cc, *md)) if m.loadingDone() && !m.spinner.Visible() { @@ -812,14 +806,6 @@ func (m *stashModel) handleDocumentBrowsing(msg tea.Msg) tea.Cmd { m.setCursor(max(0, itemsOnPage-1)) } - // If we're on the last page and we haven't loaded everything, get - // more stuff. - if m.paginator().OnLastPage() && !m.loadingFromNetwork && !m.stashFullyLoaded { - m.serverPage++ - m.loadingFromNetwork = true - cmds = append(cmds, loadStash(*m)) - } - return tea.Batch(cmds...) } @@ -973,7 +959,7 @@ func (m stashModel) view() string { case stashStateReady: loadingIndicator := " " - if !m.localOnly() && (!m.loadingDone() || m.loadingFromNetwork || m.spinner.Visible()) { + if !m.localOnly() && (!m.loadingDone() || m.spinner.Visible()) { loadingIndicator = m.spinner.View() } diff --git a/ui/ui.go b/ui/ui.go index b5542af..384b380 100644 --- a/ui/ui.go +++ b/ui/ui.go @@ -169,6 +169,14 @@ type general struct { // Local IDs of files stashed this session. We treat this like a set, // ignoring the value portion with an empty struct. filesStashed map[ksuid.KSUID]struct{} + + // Files currently being stashed. We remove files from this set once + // a stash operation has either succeeded or failed. + filesStashing map[ksuid.KSUID]struct{} +} + +func (g general) isStashing() bool { + return len(g.filesStashing) > 0 } type model struct { @@ -199,7 +207,7 @@ func (m *model) unloadDocument() []tea.Cmd { batch = append(batch, tea.ClearScrollArea) } - if !m.stash.loadingDone() || m.stash.loadingFromNetwork { + if !m.stash.loadingDone() { batch = append(batch, spinner.Tick) } return batch @@ -219,9 +227,10 @@ func newModel(cfg Config) tea.Model { } general := general{ - cfg: cfg, - authStatus: authConnecting, - filesStashed: make(map[ksuid.KSUID]struct{}), + cfg: cfg, + authStatus: authConnecting, + filesStashed: make(map[ksuid.KSUID]struct{}), + filesStashing: make(map[ksuid.KSUID]struct{}), } return model{ @@ -362,7 +371,6 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { // Even though it failed, news/stash loading is finished m.stash.loaded.Add(StashedDoc, NewsDoc) - m.stash.loadingFromNetwork = false } case keygenFailedMsg: @@ -377,7 +385,6 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { // Even though it failed, news/stash loading is finished m.stash.loaded.Add(StashedDoc, NewsDoc) - m.stash.loadingFromNetwork = false case keygenSuccessMsg: // The keygen's done, so let's try initializing the charm client again @@ -429,22 +436,20 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { cmds = append(cmds, findNextLocalFile(m)) case stashSuccessMsg: - // Something was stashed outside the file listing view. Update the - // stash listing but don't run an actual update on the stash since we - // don't want to trigger the status message and generally don't want - // any other effects. - if m.state == stateShowDocument { - md := markdown(msg) - m.stash.addMarkdowns(&md) - m.general.filesStashed[msg.localID] = struct{}{} + // Common handling that should happen regardless of application state + md := markdown(msg) + m.stash.addMarkdowns(&md) + m.general.filesStashed[msg.localID] = struct{}{} + delete(m.general.filesStashing, md.localID) - if m.stash.isFiltering() { - cmds = append(cmds, filterMarkdowns(m.stash)) - } + if m.stash.isFiltering() { + cmds = append(cmds, filterMarkdowns(m.stash)) } case stashFailMsg: + // Common handling that should happen regardless of application state delete(m.general.filesStashed, msg.markdown.localID) + delete(m.general.filesStashing, msg.markdown.localID) case filteredMarkdownMsg: if m.state == stateShowDocument {