Sync reminder notification & notification actions (#689)

Implements and closes #652.

---------

Co-authored-by: Chris Chang <51393127+chriscerie@users.noreply.github.com>
Co-authored-by: Micah <git@dekkonot.com>

CHANGELOG.md

@@ -5,12 +5,14 @@
 * Fixed the diff visualizer of connected sessions. ([#674])
 * Fixed disconnected session activity. ([#675])
 * Skip confirming patches that contain only a datamodel name change. ([#688])
+* Added sync reminder notification. ([#689])
 * Added protection against syncing a model to a place. ([#691])
 
 [#668]: https://github.com/rojo-rbx/rojo/pull/668
 [#674]: https://github.com/rojo-rbx/rojo/pull/674
 [#675]: https://github.com/rojo-rbx/rojo/pull/675
 [#688]: https://github.com/rojo-rbx/rojo/pull/688
+[#689]: https://github.com/rojo-rbx/rojo/pull/689
 [#691]: https://github.com/rojo-rbx/rojo/pull/691
 
 ## [7.3.0] - April 22, 2023

plugin/src/App/Notifications.lua

@@ -7,6 +7,7 @@ local Packages = Rojo.Packages
 
 local Roact = require(Packages.Roact)
 local Flipper = require(Packages.Flipper)
+local Log = require(Packages.Log)
 
 local bindingUtil = require(script.Parent.bindingUtil)
 
@@ -14,6 +15,7 @@ local Theme = require(Plugin.App.Theme)
 local Assets = require(Plugin.Assets)
 
 local BorderedContainer = require(Plugin.App.Components.BorderedContainer)
+local TextButton = require(Plugin.App.Components.TextButton)
 
 local baseClock = DateTime.now().UnixTimestampMillis
 
@@ -28,10 +30,8 @@ function Notification:init()
 	self.lifetime = self.props.timeout
 
 	self.motor:onStep(function(value)
-		if value <= 0 then
+		if value <= 0 and self.props.onClose then
-			if self.props.onClose then
+			self.props.onClose()
-				self.props.onClose()
-			end
 		end
 	end)
 end
@@ -86,23 +86,54 @@ function Notification:willUnmount()
 end
 
 function Notification:render()
-	local time = DateTime.fromUnixTimestampMillis(self.props.timestamp)
-
-	local textBounds = TextService:GetTextSize(
-		self.props.text,
-		15,
-		Enum.Font.GothamSemibold,
-		Vector2.new(350, 700)
-	)
-
 	local transparency = self.binding:map(function(value)
 		return 1 - value
 	end)
 
+	local textBounds = TextService:GetTextSize(
+		self.props.text,
+		15,
+		Enum.Font.GothamMedium,
+		Vector2.new(350, 700)
+	)
+
+	local actionButtons = {}
+	local buttonsX = 0
+	if self.props.actions then
+		local count = 0
+		for key, action in self.props.actions do
+			actionButtons[key] = e(TextButton, {
+				text = action.text,
+				style = action.style,
+				onClick = function()
+					local success, err = pcall(action.onClick, self)
+					if not success then
+						Log.warn("Error in notification action: " .. tostring(err))
+					end
+				end,
+				layoutOrder = -action.layoutOrder,
+				transparency = transparency,
+			})
+
+			buttonsX += TextService:GetTextSize(
+				action.text, 18, Enum.Font.GothamMedium,
+				Vector2.new(math.huge, math.huge)
+			).X + 30
+
+			count += 1
+		end
+
+		buttonsX += (count - 1) * 5
+	end
+
+	local paddingY, logoSize = 20, 32
+	local actionsY = if self.props.actions then 35 else 0
+	local contentX = math.max(textBounds.X, buttonsX)
+
 	local size = self.binding:map(function(value)
 		return UDim2.fromOffset(
-			(35+40+textBounds.X)*value,
+			(35 + 40 + contentX) * value,
-			math.max(14+20+textBounds.Y, 32+20)
+			5 + actionsY + paddingY + math.max(logoSize, textBounds.Y)
 		)
 	end)
 
@@ -122,22 +153,22 @@ function Notification:render()
 				transparency = transparency,
 				size = UDim2.new(1, 0, 1, 0),
 			}, {
-				TextContainer = e("Frame", {
+				Contents = e("Frame", {
-					Size = UDim2.new(0, 35+textBounds.X, 1, -20),
+					Size = UDim2.new(0, 35 + contentX, 1, -paddingY),
-					Position = UDim2.new(0, 0, 0, 10),
+					Position = UDim2.new(0, 0, 0, paddingY / 2),
 					BackgroundTransparency = 1
 				}, {
 					Logo = e("ImageLabel", {
 						ImageTransparency = transparency,
 						Image = Assets.Images.PluginButton,
 						BackgroundTransparency = 1,
-						Size = UDim2.new(0, 32, 0, 32),
+						Size = UDim2.new(0, logoSize, 0, logoSize),
-						Position = UDim2.new(0, 0, 0.5, 0),
+						Position = UDim2.new(0, 0, 0, 0),
-						AnchorPoint = Vector2.new(0, 0.5),
+						AnchorPoint = Vector2.new(0, 0),
 					}),
 					Info = e("TextLabel", {
 						Text = self.props.text,
-						Font = Enum.Font.GothamSemibold,
+						Font = Enum.Font.GothamMedium,
 						TextSize = 15,
 						TextColor3 = theme.Notification.InfoColor,
 						TextTransparency = transparency,
@@ -150,20 +181,21 @@ function Notification:render()
 						LayoutOrder = 1,
 						BackgroundTransparency = 1,
 					}),
-					Time = e("TextLabel", {
+					Actions = if self.props.actions then e("Frame", {
-						Text = time:FormatLocalTime("LTS", "en-us"),
+						Size = UDim2.new(1, -40, 0, 35),
-						Font = Enum.Font.Code,
+						Position = UDim2.new(1, 0, 1, 0),
-						TextSize = 12,
+						AnchorPoint = Vector2.new(1, 1),
-						TextColor3 = theme.Notification.InfoColor,
-						TextTransparency = transparency,
-						TextXAlignment = Enum.TextXAlignment.Left,
-
-						Size = UDim2.new(1, -35, 0, 14),
-						Position = UDim2.new(0, 35, 1, -14),
-
-						LayoutOrder = 1,
 						BackgroundTransparency = 1,
-					}),
+					}, {
+						Layout = e("UIListLayout", {
+							FillDirection = Enum.FillDirection.Horizontal,
+							HorizontalAlignment = Enum.HorizontalAlignment.Right,
+							VerticalAlignment = Enum.VerticalAlignment.Center,
+							SortOrder = Enum.SortOrder.LayoutOrder,
+							Padding = UDim.new(0, 5),
+						}),
+						Buttons = Roact.createFragment(actionButtons),
+					}) else nil,
 				}),
 
 				Padding = e("UIPadding", {
@@ -180,15 +212,16 @@ local Notifications = Roact.Component:extend("Notifications")
 function Notifications:render()
 	local notifs = {}
 
-	for index, notif in ipairs(self.props.notifications) do
+	for id, notif in self.props.notifications do
-		notifs[notif] = e(Notification, {
+		notifs["NotifID_" .. id] = e(Notification, {
 			soundPlayer = self.props.soundPlayer,
 			text = notif.text,
 			timestamp = notif.timestamp,
 			timeout = notif.timeout,
+			actions = notif.actions,
 			layoutOrder = (notif.timestamp - baseClock),
 			onClose = function()
-				self.props.onClose(index)
+				self.props.onClose(id)
 			end,
 		})
 	end

plugin/src/App/StatusPages/Settings/Setting.lua

@@ -59,6 +59,7 @@ function Setting:render()
 			LayoutOrder = self.props.layoutOrder,
 			ZIndex = -self.props.layoutOrder,
 			BackgroundTransparency = 1,
+			Visible = self.props.visible,
 
 			[Roact.Change.AbsoluteSize] = function(object)
 				self.setContainerSize(object.AbsoluteSize)

plugin/src/App/StatusPages/Settings/init.lua

@@ -87,19 +87,20 @@ function SettingsPage:render()
 				layoutOrder = 0,
 			}),
 
-			OpenScriptsExternally = e(Setting, {
-				id = "openScriptsExternally",
-				name = "Open Scripts Externally",
-				description = "Attempt to open scripts in an external editor",
-				transparency = self.props.transparency,
-				layoutOrder = 1,
-			}),
-
 			ShowNotifications = e(Setting, {
 				id = "showNotifications",
 				name = "Show Notifications",
 				description = "Popup notifications in viewport",
 				transparency = self.props.transparency,
+				layoutOrder = 1,
+			}),
+
+			SyncReminder = e(Setting, {
+				id = "syncReminder",
+				name = "Sync Reminder",
+				description = "Notify to sync when opening a place that has previously been synced",
+				transparency = self.props.transparency,
+				visible = Settings:getBinding("showNotifications"),
 				layoutOrder = 2,
 			}),
 
@@ -111,12 +112,20 @@ function SettingsPage:render()
 				layoutOrder = 3,
 			}),
 
+			OpenScriptsExternally = e(Setting, {
+				id = "openScriptsExternally",
+				name = "Open Scripts Externally",
+				description = "EXPERIMENTAL! Attempt to open scripts in an external editor",
+				transparency = self.props.transparency,
+				layoutOrder = 4,
+			}),
+
 			TwoWaySync = e(Setting, {
 				id = "twoWaySync",
 				name = "Two-Way Sync",
 				description = "EXPERIMENTAL! Editing files in Studio will sync them into the filesystem",
 				transparency = self.props.transparency,
-				layoutOrder = 4,
+				layoutOrder = 5,
 			}),
 
 			LogLevel = e(Setting, {
@@ -124,7 +133,7 @@ function SettingsPage:render()
 				name = "Log Level",
 				description = "Plugin output verbosity level",
 				transparency = self.props.transparency,
-				layoutOrder = 5,
+				layoutOrder = 100,
 
 				options = invertedLevels,
 				showReset = Settings:getBinding("logLevel"):map(function(value)
@@ -140,7 +149,7 @@ function SettingsPage:render()
 				name = "Typechecking",
 				description = "Toggle typechecking on the API surface",
 				transparency = self.props.transparency,
-				layoutOrder = 6,
+				layoutOrder = 101,
 			}),
 
 			Layout = e("UIListLayout", {

plugin/src/App/init.lua

@@ -1,5 +1,6 @@
 local Players = game:GetService("Players")
 local ServerStorage = game:GetService("ServerStorage")
+local RunService = game:GetService("RunService")
 
 local Rojo = script:FindFirstAncestor("Rojo")
 local Plugin = Rojo.Plugin
@@ -19,6 +20,7 @@ local ApiContext = require(Plugin.ApiContext)
 local PatchSet = require(Plugin.PatchSet)
 local preloadAssets = require(Plugin.preloadAssets)
 local soundPlayer = require(Plugin.soundPlayer)
+local ignorePlaceIds = require(Plugin.ignorePlaceIds)
 local Theme = require(script.Theme)
 
 local Page = require(script.Page)
@@ -53,6 +55,7 @@ function App:init()
 
 	self.confirmationBindable = Instance.new("BindableEvent")
 	self.confirmationEvent = self.confirmationBindable.Event
+	self.notifId = 0
 
 	self:setState({
 		appStatus = AppStatus.NotConnected,
@@ -65,28 +68,63 @@ function App:init()
 		notifications = {},
 		toolbarIcon = Assets.Images.PluginButton,
 	})
+
+	if
+		RunService:IsEdit()
+		and self.serveSession == nil
+		and Settings:get("syncReminder")
+		and self:getLastSyncTimestamp()
+	then
+		self:addNotification("You've previously synced this place. Would you like to reconnect?", 300, {
+			Connect = {
+				text = "Connect",
+				style = "Solid",
+				layoutOrder = 1,
+				onClick = function(notification)
+					notification:dismiss()
+					self:startSession()
+				end
+			},
+			Dismiss = {
+				text = "Dismiss",
+				style = "Bordered",
+				layoutOrder = 2,
+				onClick = function(notification)
+					notification:dismiss()
+				end,
+			},
+		})
+	end
 end
 
-function App:addNotification(text: string, timeout: number?)
+function App:addNotification(text: string, timeout: number?, actions: { [string]: {text: string, style: string, layoutOrder: number, onClick: (any) -> ()} }?)
 	if not Settings:get("showNotifications") then
 		return
 	end
 
+	self.notifId += 1
+	local id = self.notifId
+
 	local notifications = table.clone(self.state.notifications)
-	table.insert(notifications, {
+	notifications[id] = {
 		text = text,
 		timestamp = DateTime.now().UnixTimestampMillis,
 		timeout = timeout or 3,
-	})
+		actions = actions,
+	}
 
 	self:setState({
 		notifications = notifications,
 	})
+
+	return function()
+		self:closeNotification(id)
+	end
 end
 
-function App:closeNotification(index: number)
+function App:closeNotification(id: number)
 	local notifications = table.clone(self.state.notifications)
-	table.remove(notifications, index)
+	notifications[id] = nil
 
 	self:setState({
 		notifications = notifications,
@@ -97,12 +135,28 @@ function App:getPriorEndpoint()
 	local priorEndpoints = Settings:get("priorEndpoints")
 	if not priorEndpoints then return end
 
-	local place = priorEndpoints[tostring(game.PlaceId)]
+	local id = tostring(game.PlaceId)
+	if ignorePlaceIds[id] then return end
+
+	local place = priorEndpoints[id]
 	if not place then return end
 
 	return place.host, place.port
 end
 
+function App:getLastSyncTimestamp()
+	local priorEndpoints = Settings:get("priorEndpoints")
+	if not priorEndpoints then return end
+
+	local id = tostring(game.PlaceId)
+	if ignorePlaceIds[id] then return end
+
+	local place = priorEndpoints[id]
+	if not place then return end
+
+	return place.timestamp
+end
+
 function App:setPriorEndpoint(host: string, port: string)
 	local priorEndpoints = Settings:get("priorEndpoints")
 	if not priorEndpoints then
@@ -117,17 +171,16 @@ function App:setPriorEndpoint(host: string, port: string)
 		end
 	end
 
-	if host == Config.defaultHost and port == Config.defaultPort then
+	local id = tostring(game.PlaceId)
-		-- Don't save default
+	if ignorePlaceIds[id] then return end
-		priorEndpoints[tostring(game.PlaceId)] = nil
+
-	else
+	priorEndpoints[id] = {
-		priorEndpoints[tostring(game.PlaceId)] = {
+		host = if host ~= Config.defaultHost then host else nil,
-			host = host ~= Config.defaultHost and host or nil,
+		port = if port ~= Config.defaultPort then port else nil,
-			port = port ~= Config.defaultPort and port or nil,
+		timestamp = os.time(),
-			timestamp = os.time(),
+	}
-		}
+	Log.trace("Saved last used endpoint for {}", game.PlaceId)
-		Log.trace("Saved last used endpoint for {}", game.PlaceId)
+
-	end
 
 	Settings:set("priorEndpoints", priorEndpoints)
 end
@@ -470,7 +523,11 @@ function App:render()
 					}),
 				}),
 
-				RojoNotifications = e("ScreenGui", {}, {
+				RojoNotifications = e("ScreenGui", {
+					ZIndexBehavior = Enum.ZIndexBehavior.Sibling,
+					ResetOnSpawn = false,
+					DisplayOrder = 100,
+				}, {
 					layout = e("UIListLayout", {
 						SortOrder = Enum.SortOrder.LayoutOrder,
 						HorizontalAlignment = Enum.HorizontalAlignment.Right,
@@ -486,8 +543,8 @@ function App:render()
 					notifs = e(Notifications, {
 						soundPlayer = self.props.soundPlayer,
 						notifications = self.state.notifications,
-						onClose = function(index)
+						onClose = function(id)
-							self:closeNotification(index)
+							self:closeNotification(id)
 						end,
 					}),
 				}),

plugin/src/Settings.lua

@@ -13,6 +13,7 @@ local defaultSettings = {
 	openScriptsExternally = false,
 	twoWaySync = false,
 	showNotifications = true,
+	syncReminder = true,
 	playSounds = true,
 	typecheckingEnabled = false,
 	logLevel = "Info",

plugin/src/ignorePlaceIds.lua

@@ -0,0 +1,27 @@
+--[[
+    These are place ids that will not have metadata saved for them,
+    such as last sync address or time. This is because they are not unique
+    so storing metadata for them does not make sense as these ids are reused.
+--]]
+
+return {
+    ["0"] = true, -- Local file
+    ["95206881"] = true, -- Baseplate
+    ["6560363541"] = true, -- Classic Baseplate
+    ["95206192"] = true, -- Flat Terrain
+    ["13165709401"] = true, -- Modern City
+    ["520390648"] = true, -- Village
+    ["203810088"] = true, -- Castle
+    ["366130569"] = true, -- Suburban
+    ["215383192"] = true, -- Racing
+    ["264719325"] = true, -- Pirate Island
+    ["203812057"] = true, -- Obby
+    ["379736082"] = true, -- Starting Place
+    ["301530843"] = true, -- Line Runner
+    ["92721754"] = true, -- Capture The Flag
+    ["301529772"] = true, -- Team/FFA Arena
+    ["203885589"] = true, -- Combat
+    ["10275826693"] = true, -- Concert
+    ["5353920686"] = true, -- Move It Simulator
+    ["6936227200"] = true, -- Mansion Of Wonder
+}