Make the toolbar by Ramazanmak · Pull Request #6 · mlutcss/sandbox

Ramazanmak · 2026-01-21T13:55:02Z

No description provided.

…t sources

mr150 · 2026-01-26T15:47:50Z

src/components/tool-bar.js

toolbar в 1 слово пишется

mr150 · 2026-01-26T15:50:04Z

src/components/tool-bar.js

+
+	constructor() {
+		super();
+		this.editorKey = '';


Зачем прописывать конструктор для начальных значений, если можно их при объявлении полей класса сразу писать?

Потому что для реактивности переменной это необходимо. Если я определю реактивный атрибут через поле класса, то реактивность потеряется. Другой вопрос, правда, в том, нужна ли мне эта реактивность здесь - скорее всего нет, поэтому можно действительно в поле класса просто определить.

mr150 · 2026-01-26T15:52:55Z

src/components/tool-bar.js

+
+	render() {
+		return html`
+			<ul class="Lsst-n P0 M0 H100p D-f Ai-c Jc-fs Flg1">


Список здесь не подходит, тут ведь элементы разного типа

mr150 · 2026-01-26T15:54:13Z

src/components/tool-bar.js

+		return html`
+			<ul class="Lsst-n P0 M0 H100p D-f Ai-c Jc-fs Flg1">
+				<li class="M0 P0 Pr4u D-f Ai-str Ps -Ctx">
+					<label for="editor-key" hidden> Editor's key</label>


Нужно доступное скрытие, а не полное. И связывать с инпутом лучше вкладыванием, а не по id

mr150 · 2026-01-26T15:55:47Z

src/components/tool-bar.js

+				<li class="M0 P0 Pr4u D-f Ai-str Ps -Ctx">
+					<label for="editor-key" hidden> Editor's key</label>
+					<input name="editor-key" type="text" value="${this.editorKey}" @keyup="${this.typeKey}" placeholder="Editor's key" autocomplete="off"
+					 class="M0 P1u;3u Bdtlr1u Bdblr1u Fns4u Bd1;s;$accent800 Ol-n_f Bgc-$core800 C-$accent900_ph C-$accent800_f:ph -Ts_ph C-$accent900">


Если Ol-n_f делаем, то надо альтернативный фокус делать обязательно. Либо не надо убирать дефолтный

mr150 · 2026-01-26T15:59:58Z

src/components/tool-bar.js

+
+				</li>
+				<li class="M0 Mr4u">
+					<button class="btn D P1.5u;4u Lnh1.2 Bgc-$brand Bgc-$brand500_h C#fff Bdrd1u Txd-n C-$accent800 -Ts"> Share </button>


Это надо в алиас вынести

mr150 · 2026-01-26T16:00:22Z

index.html

  <body class="P0 M0 H100vh">
-    <header class="W100p H-$headerH Mxh18u D-g Gtc1fr;1fr P2u;5u Bgc-$core700">
+    <header class="W100p H-$headerH Mxh18u D-g Gtc1fr;1fr;1fr P2u;5u Bgc-$core700">
+


Зачем столько пустых строк?

mr150 · 2026-01-26T16:00:52Z

index.html

+						<a href="https://mlut.style/" class="Txd-n C-$accent800 C-$brand_h -Ts Fnw600"> Main</a>
+					</li>
+					<li class="M0">
+						<a href="https://mlutcss.github.io/mlut/" class="Txd-n C-$accent800 C-$brand_h -Ts Fnw600"> Docs</a>


Неверная ссылка на доку. Подлей апдейты из мастера

mr150 · 2026-01-26T16:03:37Z

src/components/tool-bar.js

+				</li>
+			</ul>
+
+			<div class="Ps-a Txa-l W20vw T-$headerH R0 Zi20 P2u Fns5u Tsd300ms"


Tsd300ms - зачем, если есть дефолтный?

И вот это как раз отдельный компонент должен быть, который мы по событию показываем. При этом, тексты ошибок в отдельным глобальном словаре лежат

mr150

!

mr150 · 2026-01-28T22:48:34Z

src/assets/scripts/client.js

@@ -0,0 +1,48 @@
+class Client {
+	static apiUrl = '';


Зачем static? Это же синглтон

mr150 · 2026-01-28T22:50:03Z

src/assets/scripts/client.js

@@ -0,0 +1,48 @@
+class Client {


По дефолту сам класс тоже экспортируем. И название надо более подходящее, а не просто client

mr150 · 2026-01-28T22:51:41Z

src/assets/scripts/client.js

+	static editorsKey = '';
+	static sketchId = '';
+
+	async makeRequest(path, method = 'GET', data = {}) {


make в названии метода здесь не подходит

mr150 · 2026-01-28T22:56:09Z

src/assets/scripts/client.js

+		return fetch(
+			`${this.apiUrl}` + path,
+			options
+		).then((resp) => {


Обработку запроса тут лучше вот такую сделать:

const result = r.headers.get('content-type').includes('application/json') ? r.json() : r.text(); if (r.ok && r.status >= 200 && r.status < 400) { return result; } throw (await result);

mr150 · 2026-01-28T22:58:16Z

src/assets/scripts/client.js

+	}
+
+	async getInitialCode(path) {
+		return await this.makeRequest(path);


Suggested change

return await this.makeRequest(path);

return this.makeRequest(path);

В любом случае промис возвращается

mr150 · 2026-01-30T12:40:16Z

src/components/error-message.js

+
+export class ErrorMessage extends LitElement {
+	static properties = {
+		errorText: { type: String },


Suggested change

errorText: { type: String },

text: { type: String },

mr150 · 2026-01-30T12:40:43Z

src/components/error-message.js

+export class ErrorMessage extends LitElement {
+	static properties = {
+		errorText: { type: String },
+		status: { type: String }


Зачем это?

mr150 · 2026-01-30T12:43:27Z

src/components/main-comp.js

+		this.isToolbarAware = false;
+
+		if (urlParams.has('art')) {
+			client.apiUrl = 'https://00017615-8bbc-455f-8757-eb63ba8c7d31.mock.pstmn.io/arts/';


Это все внутри клиента должно быть. В этом и смысл абстракции, что все детали там скрыты

mr150 · 2026-01-30T12:44:10Z

src/components/main-comp.js

+	async firstUpdated() {
+		this.isToolbarAware = false;
+
+		if (urlParams.has('art')) {


И зачем тут вообще арты? Это совершенно другая задача

mr150 · 2026-01-30T12:45:32Z

src/components/main-comp.js

+
+		try {
+			const res = await client.getInitialCode(client.sketchId);
+			[this.checkpointLayout, this.checkpointConfig] = [res.html, res.sass || '@use \'@mlut/core\';'];


Не надо делать такое присваивание. В большинстве случаев плохо читается

mr150 · 2026-01-30T12:46:14Z

src/components/main-comp.js

+			client.sketchId = urlParams.get('sketch');
+		}
+
+		try {


Асинхронную обработку ошибок лучше через чепочку промисов делать - так лучше читается

mr150 · 2026-01-30T12:48:00Z

src/components/main-comp.js

+		}
+
+		[this.currentLayout, this.currentConfig] = [this.checkpointLayout, this.checkpointConfig];
+		eventBus.emit('first-update', {


Лучше более подходящее название события, типа editor-init. И все эти названия должны быть в словаре (enum), а не строчками в коде

mr150 · 2026-01-30T12:49:46Z

src/components/main-comp.js

+			}
+		});
+
+		this.dispatchEvent(new CustomEvent('first-loaded', { bubbles: true }));


То же самое - не подходящее название

mr150 · 2026-01-30T15:56:30Z

src/components/main-comp.js

+	checkpointConfig = '';
+	currentLayout = '';
+	currentConfig = '';
+	isToolbarAware = false;


Непонятно по названию, для чего это

mr150 · 2026-01-30T15:59:34Z

src/components/main-comp.js

+	controlToolbar(layout, config) {
+		if ((layout != this.checkpointLayout || config != this.checkpointConfig)) {
+			if (!this.isToolbarAware) {
+				eventBus.emit('enable-submit', {


Лучше событие toggle-* и в payload отправляется true/false

mr150 · 2026-01-30T16:02:59Z

src/components/main-comp.js

+			console.log(client.editorsKey);
+			history.pushState(null, null, `/?sketch=${client.sketchId}`);
+
+			eventBus.emit('sketch-is-shared', {


Это не через события делается. Методы с отправкой запросов передаются в компонент тулбара и назначаются на клики по нужным кнопкам

mr150 · 2026-01-30T16:04:48Z

src/components/main-comp.js

+
+		try {
+			console.log('Share requested');
+			const res = await this.makePostRequest(event);


Зачем этот метод?

mr150 · 2026-01-30T16:06:49Z

src/assets/scripts/client.js

+		return await this.makeRequest(path);
+	}
+
+	async createNewSketch(path, method = 'POST', data) {


Тут нужен 1 метод putSketch и запрос всегда put. Он как создает, так и изменяет скетч

mr150 · 2026-01-30T16:08:11Z

src/components/error-message.js

+
+	render() {
+		return html`
+			<div id="error-wrapper" class="Ps-a Txa-l W20vw T-$headerH R0 Zi20 P2u Fns5u Tsd300ms"


text-align же всегда left?

mr150 · 2026-01-30T16:09:42Z

src/components/sandbox-toolbar.js

+import { client } from '../assets/scripts/client.js';
+import { errorTexts } from '../assets/data/error-texts';
+
+export class SandboxToolbar extends LitElement {


У нас же вроде 1 toolbar, зачем еще префикс sandbox?

mr150 · 2026-01-30T16:12:58Z

src/components/sandbox-toolbar.js

+
+			</ul>
+
+			<error-message status="${this.errorStatus}">


Я писал, что он должен в main-comp лежать и по событию вызываться

mr150 · 2026-01-30T16:16:29Z

src/components/sandbox-toolbar.js

+	};
+
+	typeKey(e) {
+		client.editorsKey = e.target.value;


Надо сделать метод у клиента setEditorsKey и через него устанавливать. Сам editorsKey - приватное свойство, как и все остальные

feat: make UI for the toolbar

33ab103

Ramazanmak requested a review from mr150 January 21, 2026 13:55

Ramazanmak self-assigned this Jan 21, 2026

Ramazanmak added 2 commits January 24, 2026 10:04

feat: add tooltip and error-message

c9c4fca

feat: add Client class and opportunity to download code from differen…

7e0f66f

…t sources

mr150 reviewed Jan 26, 2026

View reviewed changes

src/components/tool-bar.js Outdated

Copy link

Contributor

mr150 Jan 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

toolbar в 1 слово пишется

Ramazanmak reacted with thumbs up emoji

mr150 reviewed Jan 26, 2026

View reviewed changes

mr150 requested changes Jan 26, 2026

View reviewed changes

Ramazanmak and others added 2 commits January 29, 2026 01:10

feat: make a draft of the logic of sharing

2d40ff8

Merge branch 'main' into feat/toolbar

36c12c5

mr150 reviewed Jan 28, 2026

View reviewed changes

mr150 reviewed Jan 30, 2026

View reviewed changes

Ramazanmak requested a review from mr150 February 4, 2026 13:50

	return await this.makeRequest(path);
	return this.makeRequest(path);

Conversation

Ramazanmak commented Jan 21, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mr150 Jan 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mr150 Jan 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mr150 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mr150 Jan 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mr150 Jan 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mr150 Jan 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mr150 Jan 26, 2026 •

edited

Loading

mr150 Jan 26, 2026 •

edited

Loading

mr150 Jan 28, 2026 •

edited

Loading

mr150 Jan 30, 2026 •

edited

Loading

mr150 Jan 30, 2026 •

edited

Loading