Migrate DevSettings to PluginSettings for much better config flow (#572)

* Add the devsetting config options into settings

* Create dropdown component and add setting controls

* Static dropdwon width and spin arrow

* Improve dropdown option contrast and border

* Forgot to make the settings page respect the static spacing, oops

* Smaller arrow

* Vert padding

* Reset option for settings

* Hide reset button when on default

* Respect the logLevel setting

* Portal settings out to external typechecking module

* Implement new configs using the new singleton Settings

* Remove DevSettings

* Update test runner to use new settings

* More helpful test failure output

* Support non-plugin environment

* Migrate dropdown to new packages system

* Clean up components a tad

plugin/run-tests.server.lua

@@ -4,16 +4,8 @@ local TestEZ = require(ReplicatedStorage.Packages.TestEZ)
 
 local Rojo = ReplicatedStorage.Rojo
 
-local DevSettings = require(Rojo.Plugin.DevSettings)
-
-local setDevSettings = not DevSettings:hasChangedValues()
-
-if setDevSettings then
-	DevSettings:createTestSettings()
-end
+local Settings = require(Rojo.Plugin.Settings)
+Settings:set("logLevel", "Trace")
+Settings:set("typecheckingEnabled", true)
 
 require(Rojo.Plugin.runTests)(TestEZ)
-
-if setDevSettings then
-	DevSettings:resetValues()
-end

plugin/src/App/Components/Dropdown.lua

@@ -0,0 +1,169 @@
+local TextService = game:GetService("TextService")
+
+local Rojo = script:FindFirstAncestor("Rojo")
+local Plugin = Rojo.Plugin
+local Packages = Rojo.Packages
+
+local Roact = require(Packages.Roact)
+local Flipper = require(Packages.Flipper)
+
+local Assets = require(Plugin.Assets)
+local Theme = require(Plugin.App.Theme)
+local bindingUtil = require(Plugin.App.bindingUtil)
+
+local SlicedImage = require(script.Parent.SlicedImage)
+local ScrollingFrame = require(script.Parent.ScrollingFrame)
+
+local e = Roact.createElement
+
+local Dropdown = Roact.Component:extend("Dropdown")
+
+function Dropdown:init()
+	self.openMotor = Flipper.SingleMotor.new(0)
+	self.openBinding = bindingUtil.fromMotor(self.openMotor)
+
+	self.contentSize, self.setContentSize = Roact.createBinding(Vector2.new(0, 0))
+
+	self:setState({
+		open = false,
+	})
+end
+
+function Dropdown:didUpdate()
+	self.openMotor:setGoal(
+		Flipper.Spring.new(self.state.open and 1 or 0, {
+			frequency = 6,
+			dampingRatio = 1.1,
+		})
+	)
+end
+
+function Dropdown:render()
+	return Theme.with(function(theme)
+		theme = theme.Dropdown
+
+		local optionButtons = {}
+		local width = -1
+		for i, option in self.props.options do
+			local text = tostring(option or "")
+			local textSize = TextService:GetTextSize(
+				text, 15, Enum.Font.GothamMedium,
+				Vector2.new(math.huge, 20)
+			)
+			if textSize.X > width then
+				width = textSize.X
+			end
+
+			optionButtons[text] = e("TextButton", {
+				Text = text,
+				LayoutOrder = i,
+				Size = UDim2.new(1, 0, 0, 24),
+				BackgroundColor3 = theme.BackgroundColor,
+				TextTransparency = self.props.transparency,
+				BackgroundTransparency = self.props.transparency,
+				BorderSizePixel = 0,
+				TextColor3 = theme.TextColor,
+				TextXAlignment = Enum.TextXAlignment.Left,
+				TextSize = 15,
+				Font = Enum.Font.GothamMedium,
+
+				[Roact.Event.Activated] = function()
+					self:setState({
+						open = false,
+					})
+					self.props.onClick(option)
+				end,
+			}, {
+				Padding = e("UIPadding", {
+					PaddingLeft = UDim.new(0, 6),
+				}),
+			})
+		end
+
+		return e("ImageButton", {
+			Size = UDim2.new(0, width+50, 0, 28),
+			Position = self.props.position,
+			AnchorPoint = self.props.anchorPoint,
+			LayoutOrder = self.props.layoutOrder,
+			ZIndex = self.props.zIndex,
+			BackgroundTransparency = 1,
+
+			[Roact.Event.Activated] = function()
+				self:setState({
+					open = not self.state.open,
+				})
+			end,
+		}, {
+			Border = e(SlicedImage, {
+				slice = Assets.Slices.RoundedBorder,
+				color = theme.BorderColor,
+				transparency = self.props.transparency,
+				size = UDim2.new(1, 0, 1, 0),
+			}, {
+				DropArrow = e("ImageLabel", {
+					Image = Assets.Images.Dropdown.Arrow,
+					ImageColor3 = self.openBinding:map(function(a)
+						return theme.Closed.IconColor:Lerp(theme.Open.IconColor, a)
+					end),
+					ImageTransparency = self.props.transparency,
+
+					Size = UDim2.new(0, 18, 0, 18),
+					Position = UDim2.new(1, -6, 0.5, 0),
+					AnchorPoint = Vector2.new(1, 0.5),
+					Rotation = self.openBinding:map(function(a)
+						return a * 180
+					end),
+
+					BackgroundTransparency = 1,
+				}),
+				Active = e("TextLabel", {
+					Size = UDim2.new(1, -30, 1, 0),
+					Position = UDim2.new(0, 6, 0, 0),
+					BackgroundTransparency = 1,
+					Text = self.props.active,
+					Font = Enum.Font.GothamMedium,
+					TextSize = 15,
+					TextColor3 = theme.TextColor,
+					TextXAlignment = Enum.TextXAlignment.Left,
+					TextTransparency = self.props.transparency,
+				}),
+			}),
+			Options = if self.state.open then e(SlicedImage, {
+				slice = Assets.Slices.RoundedBackground,
+				color = theme.BackgroundColor,
+				position = UDim2.new(1, 0, 1, 3),
+				size = self.openBinding:map(function(a)
+					return UDim2.new(1, 0, a*math.min(3, #self.props.options), 0)
+				end),
+				anchorPoint = Vector2.new(1, 0),
+			}, {
+				Border = e(SlicedImage, {
+					slice = Assets.Slices.RoundedBorder,
+					color = theme.BorderColor,
+					transparency = self.props.transparency,
+					size = UDim2.new(1, 0, 1, 0),
+				}),
+				ScrollingFrame = e(ScrollingFrame, {
+					size = UDim2.new(1, -4, 1, -4),
+					position = UDim2.new(0, 2, 0, 2),
+					transparency = self.props.transparency,
+					contentSize = self.contentSize,
+				}, {
+					Layout = e("UIListLayout", {
+						VerticalAlignment = Enum.VerticalAlignment.Top,
+						FillDirection = Enum.FillDirection.Vertical,
+						SortOrder = Enum.SortOrder.LayoutOrder,
+						Padding = UDim.new(0, 0),
+
+						[Roact.Change.AbsoluteContentSize] = function(object)
+							self.setContentSize(object.AbsoluteContentSize)
+						end,
+					}),
+					Roact.createFragment(optionButtons),
+				}),
+			}) else nil,
+		})
+	end)
+end
+
+return Dropdown

plugin/src/App/Components/IconButton.lua

@@ -30,6 +30,7 @@ function IconButton:render()
 		Position = self.props.position,
 		AnchorPoint = self.props.anchorPoint,
 
+		Visible = self.props.visible,
 		LayoutOrder = self.props.layoutOrder,
 		ZIndex = self.props.zIndex,
 		BackgroundTransparency = 1,

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

@@ -7,9 +7,12 @@ local Packages = Rojo.Packages
 local Roact = require(Packages.Roact)
 
 local Settings = require(Plugin.Settings)
+local Assets = require(Plugin.Assets)
 local Theme = require(Plugin.App.Theme)
 
 local Checkbox = require(Plugin.App.Components.Checkbox)
+local Dropdown = require(Plugin.App.Components.Dropdown)
+local IconButton = require(Plugin.App.Components.IconButton)
 
 local e = Roact.createElement
 
@@ -54,22 +57,48 @@ function Setting:render()
 				return UDim2.new(1, 0, 0, 20 + value.Y + 20)
 			end),
 			LayoutOrder = self.props.layoutOrder,
+			ZIndex = -self.props.layoutOrder,
 			BackgroundTransparency = 1,
 
 			[Roact.Change.AbsoluteSize] = function(object)
 				self.setContainerSize(object.AbsoluteSize)
 			end,
 		}, {
-			Checkbox = e(Checkbox, {
-				active = self.state.setting,
+			Input = if self.props.options ~= nil then
+				e(Dropdown, {
+					options = self.props.options,
+					active = self.state.setting,
+					transparency = self.props.transparency,
+					position = UDim2.new(1, 0, 0.5, 0),
+					anchorPoint = Vector2.new(1, 0.5),
+					onClick = function(option)
+						Settings:set(self.props.id, option)
+					end,
+				})
+			else
+				e(Checkbox, {
+					active = self.state.setting,
+					transparency = self.props.transparency,
+					position = UDim2.new(1, 0, 0.5, 0),
+					anchorPoint = Vector2.new(1, 0.5),
+					onClick = function()
+						local currentValue = Settings:get(self.props.id)
+						Settings:set(self.props.id, not currentValue)
+					end,
+				}),
+
+			Reset = if self.props.onReset then e(IconButton, {
+				icon = Assets.Images.Icons.Reset,
+				iconSize = 24,
+				color = theme.BackButtonColor,
 				transparency = self.props.transparency,
-				position = UDim2.new(1, 0, 0.5, 0),
-				anchorPoint = Vector2.new(1, 0.5),
-				onClick = function()
-					local currentValue = Settings:get(self.props.id)
-					Settings:set(self.props.id, not currentValue)
-				end,
-			}),
+				visible = self.props.showReset,
+
+				position = UDim2.new(1, -32 - (self.props.options ~= nil and 120 or 40), 0.5, 0),
+				anchorPoint = Vector2.new(0, 0.5),
+
+				onClick = self.props.onReset,
+			}) else nil,
 
 			Text = e("Frame", {
 				Size = UDim2.new(1, 0, 1, 0),
@@ -100,11 +129,12 @@ function Setting:render()
 					TextWrapped = true,
 
 					Size = self.containerSize:map(function(value)
+						local offset = (self.props.onReset and 34 or 0) + (self.props.options ~= nil and 120 or 40)
 						local textBounds = getTextBounds(
 							self.props.description, 14, Enum.Font.Gotham, 1.2,
-							Vector2.new(value.X - 50, math.huge)
+							Vector2.new(value.X - offset, math.huge)
 						)
-						return UDim2.new(1, -50, 0, textBounds.Y)
+						return UDim2.new(1, -offset, 0, textBounds.Y)
 					end),
 
 					LayoutOrder = 2,

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

@@ -3,8 +3,10 @@ local Plugin = Rojo.Plugin
 local Packages = Rojo.Packages
 
 local Roact = require(Packages.Roact)
+local Log = require(Packages.Log)
 
 local Assets = require(Plugin.Assets)
+local Settings = require(Plugin.Settings)
 local Theme = require(Plugin.App.Theme)
 
 local IconButton = require(Plugin.App.Components.IconButton)
@@ -13,6 +15,16 @@ local Setting = require(script.Setting)
 
 local e = Roact.createElement
 
+local function invertTbl(tbl)
+	local new = {}
+	for key, value in tbl do
+		new[value] = key
+	end
+	return new
+end
+
+local invertedLevels = invertTbl(Log.Level)
+
 local function Navbar(props)
 	return Theme.with(function(theme)
 		theme = theme.Settings.Navbar
@@ -102,6 +114,30 @@ function SettingsPage:render()
 				layoutOrder = 4,
 			}),
 
+			LogLevel = e(Setting, {
+				id = "logLevel",
+				name = "Log Level",
+				description = "Plugin output verbosity level",
+				transparency = self.props.transparency,
+				layoutOrder = 5,
+
+				options = invertedLevels,
+				showReset = Settings:getBinding("logLevel"):map(function(value)
+					return value ~= "Info"
+				end),
+				onReset = function()
+					Settings:set("logLevel", "Info")
+				end,
+			}),
+
+			TypecheckingEnabled = e(Setting, {
+				id = "typecheckingEnabled",
+				name = "Typechecking",
+				description = "Toggle typechecking on the API surface",
+				transparency = self.props.transparency,
+				layoutOrder = 6,
+			}),
+
 			Layout = e("UIListLayout", {
 				FillDirection = Enum.FillDirection.Vertical,
 				SortOrder = Enum.SortOrder.LayoutOrder,

plugin/src/App/Theme.lua

@@ -72,6 +72,17 @@ local lightTheme = strict("LightTheme", {
 			BorderColor = hexColor(0xAFAFAF),
 		},
 	},
+	Dropdown = {
+		TextColor = hexColor(0x00000),
+		BorderColor = hexColor(0xAFAFAF),
+		BackgroundColor = hexColor(0xEEEEEE),
+		Open = {
+			IconColor = BRAND_COLOR,
+		},
+		Closed = {
+			IconColor = hexColor(0xEEEEEE),
+		},
+	},
 	AddressEntry = {
 		TextColor = hexColor(0x000000),
 		PlaceholderColor = hexColor(0x8C8C8C)
@@ -150,6 +161,17 @@ local darkTheme = strict("DarkTheme", {
 			BorderColor = hexColor(0x5A5A5A),
 		},
 	},
+	Dropdown = {
+		TextColor = hexColor(0xFFFFFF),
+		BorderColor = hexColor(0x5A5A5A),
+		BackgroundColor = hexColor(0x2B2B2B),
+		Open = {
+			IconColor = BRAND_COLOR,
+		},
+		Closed = {
+			IconColor = hexColor(0x484848),
+		},
+	},
 	AddressEntry = {
 		TextColor = hexColor(0xFFFFFF),
 		PlaceholderColor = hexColor(0x8B8B8B)

plugin/src/Assets.lua

@@ -23,11 +23,15 @@ local Assets = {
 		Icons = {
 			Close = "rbxassetid://6012985953",
 			Back = "rbxassetid://6017213752",
+			Reset = "rbxassetid://10142422327",
 		},
 		Checkbox = {
 			Active = "rbxassetid://6016251644",
 			Inactive = "rbxassetid://6016251963",
 		},
+		Dropdown = {
+			Arrow = "rbxassetid://10131770538",
+		},
 		Spinner = {
 			Foreground = "rbxassetid://3222731032",
 			Background = "rbxassetid://3222730627",

plugin/src/DevSettings.lua

@@ -1,139 +0,0 @@
-local Config = require(script.Parent.Config)
-
-local Environment = {
-	User = "User",
-	Dev = "Dev",
-	Test = "Test",
-}
-
-local DEFAULT_ENVIRONMENT = Config.isDevBuild and Environment.Dev or Environment.User
-
-local VALUES = {
-	LogLevel = {
-		type = "IntValue",
-		values = {
-			[Environment.User] = 2,
-			[Environment.Dev] = 4,
-			[Environment.Test] = 4,
-		},
-	},
-	TypecheckingEnabled = {
-		type = "BoolValue",
-		values = {
-			[Environment.User] = false,
-			[Environment.Dev] = true,
-			[Environment.Test] = true,
-		},
-	},
-}
-
-local CONTAINER_NAME = "RojoDevSettings" .. Config.codename
-
-local function getValueContainer()
-	return game:FindFirstChild(CONTAINER_NAME)
-end
-
-local valueContainer = getValueContainer()
-
-game.ChildAdded:Connect(function(child)
-	local success, name = pcall(function()
-		return child.Name
-	end)
-
-	if success and name == CONTAINER_NAME then
-		valueContainer = child
-	end
-end)
-
-local function getStoredValue(name)
-	if valueContainer == nil then
-		return nil
-	end
-
-	local valueObject = valueContainer:FindFirstChild(name)
-
-	if valueObject == nil then
-		return nil
-	end
-
-	return valueObject.Value
-end
-
-local function setStoredValue(name, kind, value)
-	local object = valueContainer:FindFirstChild(name)
-
-	if object == nil then
-		object = Instance.new(kind)
-		object.Name = name
-		object.Parent = valueContainer
-	end
-
-	object.Value = value
-end
-
-local function createAllValues(environment)
-	assert(Environment[environment] ~= nil, "Invalid environment")
-
-	valueContainer = getValueContainer()
-
-	if valueContainer == nil then
-		valueContainer = Instance.new("Folder")
-		valueContainer.Name = CONTAINER_NAME
-		valueContainer.Parent = game
-	end
-
-	for name, value in pairs(VALUES) do
-		setStoredValue(name, value.type, value.values[environment])
-	end
-end
-
-local function getValue(name)
-	assert(VALUES[name] ~= nil, "Invalid DevSettings name")
-
-	local stored = getStoredValue(name)
-
-	if stored ~= nil then
-		return stored
-	end
-
-	return VALUES[name].values[DEFAULT_ENVIRONMENT]
-end
-
-local DevSettings = {}
-
-function DevSettings:createDevSettings()
-	createAllValues(Environment.Dev)
-end
-
-function DevSettings:createTestSettings()
-	createAllValues(Environment.Test)
-end
-
-function DevSettings:hasChangedValues()
-	return valueContainer ~= nil
-end
-
-function DevSettings:resetValues()
-	if valueContainer then
-		valueContainer:Destroy()
-		valueContainer = nil
-	end
-end
-
-function DevSettings:isEnabled()
-	return valueContainer ~= nil
-end
-
-function DevSettings:getLogLevel()
-	return getValue("LogLevel")
-end
-
-function DevSettings:shouldTypecheck()
-	return getValue("TypecheckingEnabled")
-end
-
-function _G.ROJO_DEV_CREATE()
-	DevSettings:createDevSettings()
-end
-
-return DevSettings

plugin/src/Settings.lua

@@ -7,18 +7,22 @@ local Rojo = script:FindFirstAncestor("Rojo")
 local Packages = Rojo.Packages
 
 local Log = require(Packages.Log)
+local Roact = require(Packages.Roact)
 
 local defaultSettings = {
 	openScriptsExternally = false,
 	twoWaySync = false,
 	showNotifications = true,
 	playSounds = true,
+	typecheckingEnabled = false,
+	logLevel = "Info",
 }
 
 local Settings = {}
 
 Settings._values = table.clone(defaultSettings)
 Settings._updateListeners = {}
+Settings._bindings = {}
 
 if plugin then
 	for name, defaultValue in pairs(Settings._values) do
@@ -45,6 +49,9 @@ end
 
 function Settings:set(name, value)
 	self._values[name] = value
+	if self._bindings[name] then
+		self._bindings[name].set(value)
+	end
 
 	if plugin then
 		-- plugin:SetSetting hits disc instead of memory, so it can be slow. Spawn so we don't hang.
@@ -76,4 +83,21 @@ function Settings:onChanged(name, callback)
 	end
 end
 
+function Settings:getBinding(name)
+	local cached = self._bindings[name]
+	if cached then
+		return cached.bind
+	end
+
+	local bind, set = Roact.createBinding(self._values[name])
+	self._bindings[name] = {
+		bind = bind,
+		set = set,
+	}
+
+	Log.trace(string.format("Created binding for setting '%s'", name))
+
+	return bind
+end
+
 return Settings

plugin/src/Types.lua

@@ -1,6 +1,6 @@
 local Packages = script.Parent.Parent.Packages
 local t = require(Packages.t)
-local DevSettings = require(script.Parent.DevSettings)
+local Settings = require(script.Parent.Settings)
 local strict = require(script.Parent.strict)
 
 local RbxId = t.string
@@ -66,7 +66,7 @@ local ApiError = t.interface({
 
 local function ifEnabled(innerCheck)
 	return function(...)
-		if DevSettings:shouldTypecheck() then
+		if Settings:get("typecheckingEnabled") then
 			return innerCheck(...)
 		else
 			return true

plugin/src/init.server.lua

@@ -8,12 +8,12 @@ local Packages = Rojo.Packages
 local Log = require(Packages.Log)
 local Roact = require(Packages.Roact)
 
-local DevSettings = require(script.DevSettings)
+local Settings = require(script.Settings)
 local Config = require(script.Config)
 local App = require(script.App)
 
 Log.setLogLevelThunk(function()
-	return DevSettings:getLogLevel()
+	return Log.Level[Settings:get("logLevel")] or Log.Level.Info
 end)
 
 local app = Roact.createElement(App, {

plugin/src/init.spec.lua

@@ -2,7 +2,10 @@ return function()
 	it("should load all submodules", function()
 		local function loadRecursive(container)
 			if container:IsA("ModuleScript") and not container.Name:find("%.spec$") then
-				require(container)
+				local success, err = pcall(require, container)
+				if not success then
+					error(string.format("Failed to load '%s': %s", container.Name, err))
+				end
 			end
 
 			for _, child in ipairs(container:GetChildren()) do
@@ -12,4 +15,4 @@ return function()
 
 		loadRecursive(script.Parent)
 	end)
-end
+end