Skip to content

Conversation

cquirosj
Copy link
Contributor

@cquirosj cquirosj commented May 7, 2025

TODO:

  • improve styles for the maximize button in maximizable control
  • Test with XML

Potential improvements:

  • Extraction of the logic to load and parse a message body into a composable to be reused by Messagestore and SagaDiagramStore
  • Potential reusable component for displaying the message body, to be used in BodyView.vue and MessageDataBox.vue - Both can also be maximizable

@cquirosj cquirosj requested a review from soujay May 7, 2025 00:05
@cquirosj cquirosj marked this pull request as ready for review May 7, 2025 02:39
@cquirosj cquirosj force-pushed the data-properties branch from 704e1eb to 4903c7f Compare May 7, 2025 02:39
@cquirosj cquirosj requested review from johnsimons and PhilBastian May 7, 2025 02:41
@cquirosj cquirosj changed the title Data properties as CodeEditor Display Saga Message properties with CodeEditor May 7, 2025
@cquirosj cquirosj changed the title Display Saga Message properties with CodeEditor Display saga messages properties with CodeEditor May 7, 2025
@cquirosj cquirosj changed the title Display saga messages properties with CodeEditor Display saga messages body using CodeEditor May 7, 2025
@cquirosj cquirosj force-pushed the data-properties branch from 4903c7f to 949d699 Compare May 7, 2025 02:48
@johnsimons

This comment was marked as resolved.

Copy link
Member

@johnsimons johnsimons left a comment

Choose a reason for hiding this comment

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

Overall looks pretty good.
I am a bit unsure about us creating yet another CodeEditor though!

<div class="editor-container">
<!-- Maximize Button (shown on hover) -->
<button v-if="showMaximizeButton" @click="toggleMaximizeModal" class="maximize-button" title="Maximize view">
<img :src="DiffMaximizeIcon" alt="Maximize" width="14" height="14" />
Copy link
Member

Choose a reason for hiding this comment

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

it would be better to use the svg directly not via a img tag

Comment on lines +4 to +5
import DiffMaximizeIcon from "@/assets/diff-maximize.svg";
import DiffCloseIcon from "@/assets/diff-close.svg";
Copy link
Member

Choose a reason for hiding this comment

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

We should try to use fontawesome icons if available

export interface SagaMessageData {
message_id: string;
data: SagaMessageDataItem[];
body: DataContainer<{ value?: string; content_type?: string }>;
Copy link
Contributor

Choose a reason for hiding this comment

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

why content_type instead of contentType? It seems like this is not an API response object

};

result.body.loading = true;
result.body.failed_to_load = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

why failed_to_load and not_found rather than failedToLoad and notFound?

@cquirosj cquirosj force-pushed the data-properties branch from edf49a4 to 3145dbf Compare May 8, 2025 04:38
@cquirosj cquirosj enabled auto-merge (squash) May 8, 2025 07:16
@cquirosj cquirosj removed the request for review from soujay May 8, 2025 07:20
@cquirosj cquirosj merged commit da8f9cc into master May 8, 2025
5 checks passed
@cquirosj cquirosj deleted the data-properties branch May 8, 2025 07:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants