Skip to content

Conversation

@acelaya
Copy link
Collaborator

@acelaya acelaya commented Aug 20, 2025

Part of https://github.com/orgs/hypothesis/projects/156/views/2?pane=issue&itemId=124121878

Enhance withEditedTimestamp prop in AnnotationTimestamps component, so that it allows a variant name to be passed, customizing the "edited" text.

It still allows false and true values, which continue behaving the same (true is now equivalent to using the "default" variant).

TL;DR

This change is needed so that we can display more prominent "edited" texts in the moderation queue. I considered other options, like allowing classes to be passed, but that required an extra prop which was only relevant if withEditedTimestamp is true, or passing a list of classes to that prop, which I reckon is less intuitive than a variant name.

I also considered extracting that piece into its own component, which in turn allows classes to be provided, and let consumers compose with individual components as desired. However, the logic to render that piece is too coupled with other AnnotationTimestamps internal details, causing some duplication or having to expose too many of those details.

With that in mind I ended up with this approach, which is a bit less flexible, but very simple. We can add more variants in future if needed.

const editedTimestamp = wrapper
.find('[data-testid="timestamp-edited"]')
.getDOMNode();
assert.equal(editedTimestamp.dataset.variant, expectedVariant);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I decided to just test with a data-variant attribute, rather than testing the actual classes being added for every variant, to reduce coupling the test with implementation details.

* `subtle`: Small text with inherited font weight.
* `prominent`: Bold text with inherited font size.
*/
export type EditedTimestampVariant = 'subtle' | 'prominent';
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm not super happy with these variant names. I'm open to suggestions.

Copy link
Contributor

Choose a reason for hiding this comment

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

The intent is clear from the names, so they will work as a starting point. If you need to add more variants in future it will become clearer how to distinguish them.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok!

@acelaya acelaya merged commit 893413a into main Aug 20, 2025
2 checks passed
@acelaya acelaya deleted the edited-timestamp-variant branch August 20, 2025 08:04
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.

3 participants