Implement better behaviour for replace() on popups (#64)

monarch/monarch.lua

@@ -686,49 +686,61 @@ function M.show(id, options, data, cb)
 			local top = stack[#stack]
 			-- a screen can ignore the stack by setting the no_stack to true
 			local add_to_stack = not options or not options.no_stack
-			if add_to_stack then
+			if add_to_stack and top then
 				-- manipulate the current top
 				-- close popup(s) if needed
 				-- transition out
-				if top then
-					-- keep top popup visible if new screen can be shown on top of a popup
-					if top.popup and screen.popup_on_popup then
-						disable(top, screen)
-					else
-						-- close all popups, one by one
-						while top.popup do
-							stack[#stack] = nil
-							show_out(top, screen, callbacks.track())
-							callbacks.yield_until_done()
-							top = stack[#stack]
-						end
-						-- unload and transition out from top
-						-- wait until we are done if showing the same screen as is already visible
-						local same_screen = top and top.id == screen.id
-						show_out(top, screen, callbacks.track())
-						if same_screen or (options and options.sequential) then
-							callbacks.yield_until_done()
-						end
+				local pop = options and options.pop or 0
+				local is_not_popup = not screen.popup
+				local pop_all_popups = is_not_popup -- pop all popups when transitioning screens
+
+				-- keep top popup visible if new screen can be shown on top of a popup
+				if top.popup and screen.popup and screen.popup_on_popup then
+					disable(top, screen)
+				else
+					pop_all_popups = true
+				end
+
+				-- close popups, one by one, either all of them or the number specified by options.pop
+				while top and top.popup do
+					if not pop_all_popups then
+						if pop <= 0 then break end
+						pop = pop - 1
+					end
+					stack[#stack] = nil
+					show_out(top, screen, callbacks.track())
+					callbacks.yield_until_done()
+					top = stack[#stack]
+				end
+
+				-- unload the previous screen and transition out from top
+				-- wait until we are done if showing the same screen as is already visible
+				if top and not top.popup then
+					local same_screen = top and top.id == screen.id
+					show_out(top, screen, callbacks.track())
+					if same_screen or (options and options.sequential) then
+						callbacks.yield_until_done()
 					end
 				end
-			end
 
-			-- if the screen we want to show is in the stack
-			-- already and the clear flag is set then we need
-			-- to remove every screen on the stack up until and
-			-- including the screen itself
-			if options and options.clear then
-				log("show() clearing")
-				while M.in_stack(id) do
-					table.remove(stack)
+				-- if the screen we want to show is in the stack
+				-- already and the clear flag is set then we need
+				-- to remove every screen on the stack up until and
+				-- including the screen itself
+				if options and options.clear then
+					log("show() clearing")
+					while M.in_stack(id) do
+						table.remove(stack)
+					end
 				end
-			end
 
-			if options and options.pop then
-				for i = 1, options.pop do
-					local stack_top = #stack
-					if stack_top < 1 then break end
-					stack[stack_top] = nil
+				-- pop screens off the stack
+				if is_not_popup then
+					for i = 1, pop do
+						local stack_top = #stack
+						if stack_top < 1 then break end
+						stack[stack_top] = nil
+					end
 				end
 			end
 

test/test_monarch.lua

@@ -329,10 +329,49 @@ return function()
 			assert(wait_until_shown(POPUP2), "Popup2 was never shown")
 			assert_stack({ SCREEN1, POPUP1, POPUP2 })
 			monarch.replace(SCREEN2)
-			assert(wait_until_shown(SCREEN2), "Popup2 was never shown")
+			assert(wait_until_shown(SCREEN2), "Screen2 was never shown")
 			assert_stack({ SCREEN2 })
 		end)
 
+		it("should replace a popup", function()
+			monarch.show(SCREEN1)
+			assert(wait_until_shown(SCREEN1), "Screen1 was never shown")
+			assert_stack({ SCREEN1 })
+			monarch.show(POPUP1)
+			assert(wait_until_shown(POPUP1), "Popup1 was never shown")
+			assert_stack({ SCREEN1, POPUP1 })
+			monarch.replace(POPUP2)
+			assert(wait_until_shown(POPUP2), "Popup2 was never shown")
+			assert_stack({ SCREEN1, POPUP2 })
+		end)
+
+		it("should replace a pop-up two levels down", function()
+			monarch.show(SCREEN1)
+			assert(wait_until_shown(SCREEN1), "Screen1 was never shown")
+			assert_stack({ SCREEN1 })
+			monarch.show(POPUP1)
+			assert(wait_until_shown(POPUP1), "Popup1 was never shown")
+			assert_stack({ SCREEN1, POPUP1 })
+			monarch.show(POPUP2)
+			assert(wait_until_shown(POPUP2), "Popup2 was never shown")
+			assert_stack({ SCREEN1, POPUP1, POPUP2 })
+			monarch.show(POPUP2, { pop = 2 })
+			assert(wait_until_shown(POPUP2), "Popup2 was never shown")
+			assert_stack({ SCREEN1, POPUP2 })
+		end)
+
+		it("shouldn't over-pop popups", function()
+			monarch.show(SCREEN1)
+			assert(wait_until_shown(SCREEN1), "Screen1 was never shown")
+			assert_stack({ SCREEN1 })
+			monarch.show(POPUP1)
+			assert(wait_until_shown(POPUP1), "Popup1 was never shown")
+			assert_stack({ SCREEN1, POPUP1 })
+			monarch.show(POPUP2, { pop = 10 })
+			assert(wait_until_shown(POPUP2), "Popup2 was never shown")
+			assert_stack({ SCREEN1, POPUP2 })
+		end)
+
 		it("should be able to get the id of the screen at the top and bottom of the stack", function()
 			assert(monarch.top() == nil)
 			assert(monarch.bottom() == nil)