Skip to content

Conversation

alserene
Copy link
Contributor

@alserene alserene commented Sep 16, 2025

PR for the Context Feed :)

Please merge the related backend PR first - #75.

Notes:

  • There is a long list of features that could still be implemented. These will be documented in the Jira ticket and considered alongside any user requests.
  • Default view shows either Simonyi or AuxTel Exposures, MTQueue or ATQueue, and the relevant Narrative Logs. The narrative log split I added is likely going to come from rubin_nights in the future, but for now, we'll do it in the front end.
  • The colour palette doesn't need to be colourblind friendly, as it serves as an aid in reading the table data. I have attempted to make some sort of colour grouping per telescope (as suggested here) but it isn't quite right and looks ugly, so I've kept it out for now.

@alserene alserene changed the title Tickets/OSW-666 Tickets/OSW-666: Implement Context Feed Sep 19, 2025
@alserene alserene changed the title Tickets/OSW-666: Implement Context Feed OSW-666: Implement Context Feed Sep 21, 2025
Copy link
Contributor

@Vebop Vebop left a comment

Choose a reason for hiding this comment

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

I left a number of comments, but since I'm still new to JS I would prefer someone else's experienced approval, over my noticing of whitespace and small typos.
It feels like a shallow: LGTM

Copy link
Contributor

@sebastian-aranda sebastian-aranda left a comment

Choose a reason for hiding this comment

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

Very nice job @alserene 👏 ! This looks good, I left several comments and ideas for your consideration, but not strictly change request unless you agree with those 👍.

I have to say though that there were way too many comments for my taste 😅. I think there are some cases where you would require a more detailed comment e.g.:

Image

And then we start thinking if having some docs for that would be better, but for now this is ok, is not necessary to re arrange them.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this one would be benefited of some refactoring in base to: https://react.dev/learn/you-might-not-need-an-effect, specifically by using what's detailed on Resetting all state when a prop changes.

@alserene alserene force-pushed the tickets/OSW-666 branch 2 times, most recently from f611a86 to 3586660 Compare September 25, 2025 15:20
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