feat: CP-11851 Core concierge #432
Conversation
vvava
changed the title
|
Feature flags are missing |
meeh0w
left a comment
meeh0w
left a comment
There was a problem hiding this comment.
I noted the only comment that actually requires an action for me to approve the PR, please let me know once done 👍
There was a problem hiding this comment.
Action required: Please create a follow-up ticket to break this down into separate files / hooks. It's very hard to parse this file in its current shape.
There was a problem hiding this comment.
| text={message.content} | ||
| scrollToBottom={scrollToBottom} | ||
| typingSpeed={typingSpeed} |
There was a problem hiding this comment.
But the text length is clearly the single thing it actually cares about when it comes to the typing speed. It's not business logic, it's the presentation logic.
And tbh I think deriving the typing speed based on the text length is the only thing that makes sense, otherwise the component would become super clunky imho (I mean, it's good that longer text translates into higher typing speed).
The only other way I could see it be is that the message typing duration could always be constant (e.g. no matter the content, it would always take 1s for the message to appear). But I feel like this could also not be super user-friendly.
Just saying, leave it be if you wish -- no action required.
| 'Get Core to work for you. Whether it’s transferring, sending crypto, just ask away!', | ||
| )} | ||
| checked={coreAssistant} | ||
| onChange={() => setCoreAssistant(!coreAssistant)} |
There was a problem hiding this comment.
You should use a state setter's callback when the new state depends on the previous state, such as incrementing a counter or reversing boolean flags. Using the callback ensures your update is based on the most current state, preventing issues from React's state update batching and potential race conditions.
Low risk here, but it's a good thing to remember especially with more complex state variables (e.g. objects, arrays). Also useful inside useEffect or useCallback - you don't need the state itself as a dependency.
frichards
left a comment
frichards
left a comment
There was a problem hiding this comment.
🌈 🦄 Approved!
Manually tested
| '&.backdrop-enter': { | ||
| display: 'flex', | ||
| }, | ||
| '&.backdrop-enter-done': { | ||
| display: 'flex', |
There was a problem hiding this comment.
Please replace class name literals with constants to have a link between the selectors and the logic applying the CSS classes
Description
Implement the Concierge with the new UI.
Changes
Put the button and the animation to the header in the portfolio page. Create the Concierge page.
Testing
Go to Concierge page and play around with it. Also try the turn on and off in settings and feature flag out.
Screenshots:
Screen.Recording.2025-09-22.at.9.47.02.mov
Screen.Recording.2025-09-22.at.9.47.59.mov
Checklist for the author
Tick each of them when done or if not applicable.