Skip to content

Conversation

ajweeks
Copy link

@ajweeks ajweeks commented Apr 18, 2025

This PR adds support for leaving a comment in each reflection.

A new serialization version has been created to handle storing the additional string data, v3, which is largely a copy of v2. Small modifications were made to encodeEntryData.

The remaining work is primarily improving the styling of the comment field, on both the input / output pages.

Copy link
Owner

@Greenheart Greenheart left a comment

Choose a reason for hiding this comment

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

Great work so far on this! 🌟

Really good how you solved encoding of the variable length strings, and implemented the core functionality.

Apart from some minor comments to resolve, the next step is to align the styling with the other input fields. One source of inspiration might be to look at the other input fields and copy the same classes to get matching borders, colours, backgrounds, paddings and similar.

{#if currentReflection.comment != null && currentReflection.comment.length > 0}
<CommentView textData={currentReflection.comment} />
{:else}
<CommentView textData="." hidden />
Copy link
Author

Choose a reason for hiding this comment

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

This is a bit of a hacky solution to get a non-empty string to appear in the component so it gets the same height as reflections with comments. I've set the visibility to hidden so that when scrolling between reflections the page layout remains consistent. This solution can surely be improved though.

This commit also prevents arrow key navigation between pages when editing a comment
@ajweeks ajweeks force-pushed the feature/add-comment-support branch from 527246c to 606f975 Compare June 9, 2025 13:34
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.

2 participants