Modal fixes (#281) (#284)

Author: awfulcooking

display/display.go

@@ -241,7 +241,7 @@ func Init(version, commit, builtBy string) {
 			}
 			if i <= len(tabs[tab].page.Links) && i > 0 {
 				// It's a valid link number
-				followLink(tabs[tab], tabs[tab].page.URL, tabs[tab].page.Links[i-1])
+				go followLink(tabs[tab], tabs[tab].page.URL, tabs[tab].page.Links[i-1])
 				return
 			}
 			// Invalid link number, don't do anything

display/handlers.go

@@ -352,15 +352,15 @@ func handleURL(t *tab, u string, numRedirects int) (string, bool) {
 			// Disable read timeout and go back to start
 			res.SetReadTimeout(0) //nolint: errcheck
 			res.Body.(*rr.RestartReader).Restart()
-			go dlChoice("That page is too large. What would you like to do?", u, res)
+			dlChoice("That page is too large. What would you like to do?", u, res)
 			return ret("", false)
 		}
 		if errors.Is(err, renderer.ErrTimedOut) {
 			// Downloading now
 			// Disable read timeout and go back to start
 			res.SetReadTimeout(0) //nolint: errcheck
 			res.Body.(*rr.RestartReader).Restart()
-			go dlChoice("Loading that page timed out. What would you like to do?", u, res)
+			dlChoice("Loading that page timed out. What would you like to do?", u, res)
 			return ret("", false)
 		}
 		if err != nil {
@@ -493,7 +493,7 @@ func handleURL(t *tab, u string, numRedirects int) (string, bool) {
 				// Disable read timeout and go back to start
 				res.SetReadTimeout(0) //nolint: errcheck
 				res.Body.(*rr.RestartReader).Restart()
-				go dlChoice("That file could not be displayed. What would you like to do?", u, res)
+				dlChoice("That file could not be displayed. What would you like to do?", u, res)
 			}
 		}()
 		return ret("", false)
@@ -503,6 +503,6 @@ func handleURL(t *tab, u string, numRedirects int) (string, bool) {
 	// Disable read timeout and go back to start
 	res.SetReadTimeout(0) //nolint: errcheck
 	res.Body.(*rr.RestartReader).Restart()
-	go dlChoice("That file could not be displayed. What would you like to do?", u, res)
+	dlChoice("That file could not be displayed. What would you like to do?", u, res)
 	return ret("", false)
 }

display/modals.go

@@ -16,24 +16,21 @@ import (
 // The bookmark modal is in bookmarks.go
 
 var infoModal = cview.NewModal()
-
 var errorModal = cview.NewModal()
-var errorModalDone = make(chan struct{})
-
 var inputModal = cview.NewModal()
-var inputCh = make(chan string)
-var inputModalText string // The current text of the input field in the modal
-
 var yesNoModal = cview.NewModal()
 
-// Channel to receive yesNo answer on
+var inputCh = make(chan string)
 var yesNoCh = make(chan bool)
 
+var inputModalText string // The current text of the input field in the modal
+
+// Internal channel used to know when a modal has been dismissed
+var modalDone = make(chan struct{})
+
 func modalInit() {
 	infoModal.AddButtons([]string{"Ok"})
-
 	errorModal.AddButtons([]string{"Ok"})
-
 	yesNoModal.AddButtons([]string{"Yes", "No"})
 
 	panels.AddPanel(PanelInfoModal, infoModal, false, false)
@@ -145,6 +142,7 @@ func modalInit() {
 		panels.HidePanel(PanelInfoModal)
 		App.SetFocus(tabs[curTab].view)
 		App.Draw()
+		modalDone <- struct{}{}
 	})
 
 	errorModal.SetBorder(true)
@@ -153,7 +151,7 @@ func modalInit() {
 		panels.HidePanel(PanelErrorModal)
 		App.SetFocus(tabs[curTab].view)
 		App.Draw()
-		errorModalDone <- struct{}{}
+		modalDone <- struct{}{}
 	})
 
 	inputModal.SetBorder(true)
@@ -183,7 +181,7 @@ func modalInit() {
 	dlInit()
 }
 
-// Error displays an error on the screen in a modal.
+// Error displays an error on the screen in a modal, and blocks until dismissed by the user.
 func Error(title, text string) {
 	if text == "" {
 		text = "No additional information."
@@ -203,19 +201,21 @@ func Error(title, text string) {
 	App.SetFocus(errorModal)
 	App.Draw()
 
-	<-errorModalDone
+	<-modalDone
 }
 
-// Info displays some info on the screen in a modal.
+// Info displays some info on the screen in a modal, and blocks until dismissed by the user.
 func Info(s string) {
 	infoModal.SetText(s)
 	panels.ShowPanel(PanelInfoModal)
 	panels.SendToFront(PanelInfoModal)
 	App.SetFocus(infoModal)
 	App.Draw()
+
+	<-modalDone
 }
 
-// Input pulls up a modal that asks for input, and returns the user's input.
+// Input pulls up a modal that asks for input, waits for that input, and returns it.
 // It returns an bool indicating if the user chose to send input or not.
 func Input(prompt string, sensitive bool) (string, bool) {
 	// Remove elements and re-add them - to clear input text and keep input in focus
@@ -257,7 +257,7 @@ func Input(prompt string, sensitive bool) (string, bool) {
 	return resp, true
 }
 
-// YesNo displays a modal asking a yes-or-no question.
+// YesNo displays a modal asking a yes-or-no question, waits for an answer, then returns it as a bool.
 func YesNo(prompt string) bool {
 	if viper.GetBool("a-general.color") {
 		m := yesNoModal
@@ -289,7 +289,7 @@ func YesNo(prompt string) bool {
 }
 
 // Tofu displays the TOFU warning modal.
-// It returns a bool indicating whether the user wants to continue.
+// It blocks then returns a bool indicating whether the user wants to continue.
 func Tofu(host string, expiry time.Time) bool {
 	// Reuses yesNoModal, with error color
 

display/private.go

@@ -1,5 +1,8 @@
 package display
 
+// This file contains the functions that aren't part of the public API.
+// The funcs are for network and displaying.
+
 import (
 	"net/url"
 	"strconv"
@@ -9,17 +12,20 @@ import (
 	"github.com/makeworld-the-better-one/amfora/structs"
 )
 
-// This file contains the functions that aren't part of the public API.
-// The funcs are for network and displaying.
-
-// followLink should be used when the user "clicks" a link on a page.
-// Not when a URL is opened on a new tab for the first time.
-// It will handle setting the bottomBar.
+// followLink should be used when the user "clicks" a link on a page,
+// but not when a URL is opened on a new tab for the first time.
+//
+// It will handle updating the bottomBar.
+//
+// It should be called with the `go` keyword to spawn a new goroutine if
+// it would otherwise block the UI loop, such as when called from an input
+// handler.
+//
+// It blocks until navigation is finished, and we've completed any user
+// interaction related to loading the URL (such as info, error modals)
 func followLink(t *tab, prev, next string) {
 	if strings.HasPrefix(next, "about:") {
-		if final, ok := handleAbout(t, next); ok {
-			t.addToHistory(final)
-		}
+		goURL(t, next)
 		return
 	}
 
@@ -29,7 +35,7 @@ func followLink(t *tab, prev, next string) {
 			Error("URL Error", err.Error())
 			return
 		}
-		go goURL(t, nextURL)
+		goURL(t, nextURL)
 		return
 	}
 	// No content on current tab, so the "prev" URL is not valid.
@@ -39,7 +45,7 @@ func followLink(t *tab, prev, next string) {
 		Error("URL Error", "Link URL could not be parsed")
 		return
 	}
-	go goURL(t, next)
+	goURL(t, next)
 }
 
 // reformatPage will take the raw page content and reformat it according to the current terminal dimensions.

display/tab.go

@@ -86,7 +86,7 @@ func makeNewTab() *tab {
 			linkN, _ := strconv.Atoi(currentSelection[0])
 			tabs[tab].page.Selected = tabs[tab].page.Links[linkN]
 			tabs[tab].page.SelectedID = currentSelection[0]
-			followLink(tabs[tab], tabs[tab].page.URL, tabs[tab].page.Links[linkN])
+			go followLink(tabs[tab], tabs[tab].page.URL, tabs[tab].page.Links[linkN])
 			return
 		}
 		if len(currentSelection) == 0 && (key == tcell.KeyEnter || key == tcell.KeyTab) {
@@ -156,12 +156,12 @@ func makeNewTab() *tab {
 			if t.hasContent() {
 				savePath, err := downloadPage(t.page)
 				if err != nil {
-					Error("Download Error", fmt.Sprintf("Error saving page content: %v", err))
+					go Error("Download Error", fmt.Sprintf("Error saving page content: %v", err))
 				} else {
-					Info(fmt.Sprintf("Page content saved to %s. ", savePath))
+					go Info(fmt.Sprintf("Page content saved to %s. ", savePath))
 				}
 			} else {
-				Info("The current page has no content, so it couldn't be downloaded.")
+				go Info("The current page has no content, so it couldn't be downloaded.")
 			}
 			return nil
 		case config.CmdBack:
@@ -178,7 +178,7 @@ func makeNewTab() *tab {
 			currentURL := tabs[curTab].page.URL
 			err := clipboard.WriteAll(currentURL)
 			if err != nil {
-				Error("Copy Error", err.Error())
+				go Error("Copy Error", err.Error())
 				return nil
 			}
 			return nil
@@ -193,14 +193,14 @@ func makeNewTab() *tab {
 			if err != nil {
 				err := clipboard.WriteAll(selectedURL)
 				if err != nil {
-					Error("Copy Error", err.Error())
+					go Error("Copy Error", err.Error())
 					return nil
 				}
 				return nil
 			}
 			err = clipboard.WriteAll(copiedURL.String())
 			if err != nil {
-				Error("Copy Error", err.Error())
+				go Error("Copy Error", err.Error())
 				return nil
 			}
 			return nil
@@ -209,7 +209,7 @@ func makeNewTab() *tab {
 		if cmd >= config.CmdLink1 && cmd <= config.CmdLink0 {
 			if int(cmd) <= len(t.page.Links) {
 				// It's a valid link number
-				followLink(&t, t.page.URL, t.page.Links[cmd-1])
+				go followLink(&t, t.page.URL, t.page.Links[cmd-1])
 				return nil
 			}
 		}