Skip to content

Conversation

ArtemHoruzhenko
Copy link
Collaborator

…onses message placeholder

egor-zalenski
egor-zalenski previously approved these changes Dec 12, 2024
)}
</>
)}
{isSizeLimitExceededResponse(result)
Copy link
Collaborator

Choose a reason for hiding this comment

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

A lot of nesting 🥲

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah. Otherwise we must split/refactor entire component which is time-consuming for such small task.

Copy link
Contributor

Choose a reason for hiding this comment

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

If any of notices such deep nesting and ternaries being used in the rendering, or anything else that might be a suboptimal practice, but it is done to get it done for now. Please, tag me, so we can discuss it and make a task for it to improve it whenever we have time. Otherwise we'll forget about it and things like these or similar will keep falling through the cracks.
It will also be more productive than emotes ;)
Tagging @pd-redis and @seth-riedel so they can see it as well

Copy link
Collaborator

Choose a reason for hiding this comment

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

why not use

(isGroupResults(resultsMode) || isSizeLimitExceededResponse(result)) ? ...

Copy link
Collaborator

Choose a reason for hiding this comment

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

Love the initiative, @KIvanow! While I’m slightly skeptical about the 'whenever we have time' part, it’s definitely a step forward. And hey, emojis might be simple, but they’ve got some serious discussion-starting power! 😸✨

@KIvanow
Copy link
Contributor

KIvanow commented Dec 13, 2024

Please start the commits with the ticket id so we can more easily associate the changes with what caused them whenever we need to change anything later on

@mariasergeenko mariasergeenko merged commit 0015ce4 into main Dec 30, 2024
12 checks passed
@mariasergeenko mariasergeenko deleted the feature/RI-6323-wb-big-responses branch December 30, 2024 14:08
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.

6 participants