Added editMessage function #9

JaBrik228 · 2024-07-30T18:33:52Z

No description provided.

SDesya74 · 2024-07-30T18:45:37Z

src/lib/assets/controlIcons/editIcon.svg

@@ -0,0 +1,13 @@
+<?xml version="1.0" encoding="iso-8859-1"?>


Прогони через svg optimizer чтобы убрал комменты и лишнее

SDesya74 · 2024-07-30T18:57:47Z

src/lib/index.svelte.ts

 }

-export type Message = {
+export interface IMessage {


Не уверен что следует слой логики переименовывать ради фронтенда

SDesya74 · 2024-07-30T18:58:59Z

src/lib/index.svelte.ts

        delete this.inner[id]
    }

+    edit(id: Id, newContent: string) {


А почему бы просто сообщению контент не установить, раз оно в onedit доступно? Реактивное же

SDesya74 · 2024-07-30T19:00:29Z

src/routes/+page.svelte

    }
+
+    function onEditMessage() {
+        if (currEditableMessageId) {


Условие можно инвертировать, тогда вложенность будет меньше

if (!currEditableMessageId) { return }

И имя currEditableMessageId наверное лучше заменить на currentEditingMessage, возможно даже лучше сделать не Id, потому что message - сигнал и по Id его можно не таскать

SDesya74 · 2024-07-30T19:03:29Z

src/routes/+page.svelte

+        }
+    }
+
+    function focusOnMessage(value: IMessage) {


function startEditing мб? Более декларативно звучит и объясняет, зачем нужна функция

SDesya74 · 2024-07-30T19:04:49Z

src/routes/+page.svelte

+        textInputElement?.focus()
+    }
+
+    function onEditMessageByKeyDown(e: KeyboardEvent) {


Та же история, startEditByShortcut или типа того, лучше именовать бизнес функции по смыслу, а не по действию

SDesya74 · 2024-07-30T19:07:48Z

src/routes/+page.svelte


    <footer
-        class="dark:text-white bg-slate-100 dark:bg-slate-700 border-t border-gray-300 dark:border-slate-600 p-4 absolute left-0 bottom-0 w-full"
+        class={`dark:text-white ${currEditableMessageId ? 'bg-orange-100' : 'bg-slate-100'} dark:bg-slate-700 border-t border-gray-300 dark:border-slate-600 p-4 absolute left-0 bottom-0 w-full`}


Тут можно воспользоваться свелтовским синтаксисом

class:bg-orange-100={currEditableMessageId}

А, ну и bg-orange возможно стоит поменять на bg-accent с прозрачностью через bg-opacity. Хотя текущая механика установки цветов прозрачность не поддерживает... В целом можно и bg-orange оставить, но если есть желание прямо в этом ПР починить и прозрачность цветов, можешь это сделать. В файле со стилями темы вроде даже написано как

JaBrik228 · 2024-07-30T22:30:42Z

Нашёл баг: При изменении сообщения, оно не обновлялось в localStorage.
Добавил функцию, в которой происходит запись сообщений в localStorage, чтобы не дублировать код в двух местах, теперь если логика сохранения сообщений измениться, можно будет изменить её в одной функции.

Так же внёс все правки, по твоим замечаниям

SDesya74 · 2024-07-31T02:43:23Z

src/lib/index.svelte.ts

        delete this.inner[id]
    }

+    edit(message: Message, newContent: string) {


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

SDesya74 · 2024-07-31T02:44:50Z

src/routes/+page.svelte

    // TODO: Make a multiline chat input that grows dynamically
    let textInputElement: HTMLInputElement | null = $state(null)

+    function saveMessagesOnLocalStorage() {


saveMessagesToLocalStorage

SDesya74 · 2024-07-31T02:45:41Z

src/routes/+page.svelte

+        currentEditingMessage = null
+        textInput = ''
+
+        setTimeout(() => textInputElement?.focus(), 0)


Лучше поменять на tick, в svelte есть

SDesya74 · 2024-07-31T02:46:31Z

src/routes/+page.svelte

+        textInputElement?.focus()
+    }
+
+    function startEditByShortcut(e: KeyboardEvent) {


startEditingByShortcut, как в функции выше

SDesya74 · 2024-07-31T02:49:29Z

src/routes/+page.svelte


    <footer
-        class="dark:text-white bg-slate-100 dark:bg-slate-700 border-t border-gray-300 dark:border-slate-600 p-4 absolute left-0 bottom-0 w-full"
+        class:bg-orange-100={currentEditingMessage}


Необязательно описывать оба варианта, можно в class описать ситуацию по умолчанию, когда !currentEditingMessage, а потом под ним описать дополнение с class:bg-orange-100. Оранжевый может оверрайдить серый, а приоритет у него будет выше, поскольку он указывается после дефолтного. В такой реализации пропадёт дублирование проверки currentEditingMessage

Я изначально так и сделал, это не сработало

Added editMessage function

2cb55f3

JaBrik228 requested a review from SDesya74 July 30, 2024 18:33

SDesya74 requested changes Jul 30, 2024

View reviewed changes

fixed some issues

7c195fb

SDesya74 requested changes Jul 31, 2024

View reviewed changes

Added editMessage function #9

Are you sure you want to change the base?

Added editMessage function #9

Uh oh!

Conversation

JaBrik228 commented Jul 30, 2024

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!

JaBrik228 commented Jul 30, 2024

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants