Repository: GraphiteEditor/Graphite
File: frontend/src/components/widgets/options/ToolOptions.vue (Vue)

Code before:
import NumberInput from "@/components/widgets/inputs/NumberInput.vue";

const wasm = import("@/../wasm/pkg");

export default defineComponent({
	props: {
		activeTool: { type: String },
		currentToolOptions: { type: Object },
	},
	computed: {
		activeToolOptions(): Record<string, object> {
			return (this.$props.currentToolOptions || {})[this.$props.activeTool || ""];
		},
	},
	methods: {
		async updateToolOptions(path: string[], newValue: number) {
			this.setToolOption(path, newValue);
			(await wasm).set_tool_options(this.$props.activeTool || "", this.activeToolOptions);
		},
		async sendToolMessage(message: string | object) {
			(await wasm).send_tool_message(this.$props.activeTool || "", message);
		},
		// Traverses the given path into the active tool's option struct, and sets the value at the path tail
		setToolOption(path: string[], newValue: number) {
			let value = this.activeToolOptions as Record<string, object | number>;
			[this.$props.activeTool || "", ...path.slice(0, -1)].forEach((attr) => {
				value = value[attr] as Record<string, object | number>;
			});
			const final = value as Record<string, number>;
			final[path.slice(-1)[0]] = newValue;
		},
		// Traverses the given path into the active tool's option struct, and returns the value at the path tail
		getToolOption(path: string[]): number {
			let value = this.activeToolOptions as Record<string, object | number>;
			[this.$props.activeTool || "", ...path.slice(0, -1)].forEach((attr) => {
				value = value[attr] as Record<string, object | number>;
			});
			const final = value as Record<string, number>;
			return final[path.slice(-1)[0]];
		},
		handleIconButtonAction(option: IconButtonWidget) {
			if (option.message) {
				this.sendToolMessage(option.message);
				return;
			}

			if (option.callback) {
				option.callback();
				return;
			}


Review comment (suggestion):
I don't remember off the top of my head what `.slice(0, -1)` does, so breaking this out into a `const` variable above with a self-descriptive name would be helpful for readability.

Can this `.forEach()` loop that mutates `value` be turned into a `.map()` which generates a new array, and then it gets combined with `value` with a spread operator when defining `final`?

More usage of self-documenting variable names, or otherwise some descriptive comments, would be really helpful in this function which isn't trivial to read through. Thanks.

Code after:
import PopoverButton from "@/components/widgets/buttons/PopoverButton.vue";
import NumberInput from "@/components/widgets/inputs/NumberInput.vue";

const wasm = import("@/../wasm/pkg");

export default defineComponent({
	props: {
		activeTool: { type: String },
		activeToolOptions: { type: Object as PropType<Record<string, object>> },
	},
	methods: {
		async updateToolOptions(path: string[], newValue: number) {
			this.setToolOption(path, newValue);
			(await wasm).set_tool_options(this.activeTool || "", this.activeToolOptions);
		},
		async sendToolMessage(message: string | object) {
			(await wasm).send_tool_message(this.activeTool || "", message);
		},
		// Traverses the given path and returns the direct parent of the option
		getRecordContainingOption(optionPath: string[]): Record<string, number> {
			const allButLast = optionPath.slice(0, -1);
			let currentRecord = this.activeToolOptions as Record<string, object | number>;
			[this.activeTool || "", ...allButLast].forEach((attr) => {
				currentRecord = currentRecord[attr] as Record<string, object | number>;
			});
			return currentRecord as Record<string, number>;
		},
		// Traverses the given path into the active tool's option struct, and sets the value at the path tail
		setToolOption(optionPath: string[], newValue: number) {
			const last = optionPath.slice(-1)[0];
			const recordContainingOption = this.getRecordContainingOption(optionPath);
			recordContainingOption[last] = newValue;
		},
		// Traverses the given path into the active tool's option struct, and returns the value at the path tail
		getToolOption(optionPath: string[]): number {
			const last = optionPath.slice(-1)[0];
			const recordContainingOption = this.getRecordContainingOption(optionPath);
			return recordContainingOption[last];
		},
		handleIconButtonAction(option: IconButtonWidget) {
			if (option.message) {
				this.sendToolMessage(option.message);
				return;
			}

			if (option.callback) {
				option.callback();
				return;
			}

			comingSoon();