feat(orama): use new UI components #7971
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
avivkeller
changed the title
Co-authored-by: Aviv Keller <me@aviv.sh> Signed-off-by: Aileen Villanueva Lecuona <aileenvl@gmail.com>
Co-authored-by: Aviv Keller <me@aviv.sh> Signed-off-by: Aileen Villanueva Lecuona <aileenvl@gmail.com>
Co-authored-by: Aviv Keller <me@aviv.sh> Signed-off-by: Aileen Villanueva Lecuona <aileenvl@gmail.com>
Co-authored-by: Aviv Keller <me@aviv.sh> Signed-off-by: Aileen Villanueva Lecuona <aileenvl@gmail.com>
Co-authored-by: Aviv Keller <me@aviv.sh> Signed-off-by: Aileen Villanueva Lecuona <aileenvl@gmail.com>
|
Note Your Pull Request seems to be updating Translations of the Node.js Website. Whilst we appreciate your intent; Any Translation update should be done through our Crowdin Project. Thank you! |
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #7971 +/- ##
==========================================
- Coverage 73.73% 73.13% -0.60%
==========================================
Files 96 95 -1
Lines 8365 8354 -11
Branches 219 218 -1
==========================================
- Hits 6168 6110 -58
- Misses 2196 2243 +47
Partials 1 1 ☔ View full report in Codecov by Sentry. |
|
Don't worry about the Playwright failures right now. Once this is ready for review, I'll push a commit updating the tests/component to interact with each other. |
| const collectionManager = new CollectionManager({ | ||
| authJwtURL: 'https://staging.app.orama.com/api/user/jwt', | ||
| collectionID: '85f541b3-b691-4d3e-9874-e7b3b4630adb', | ||
| apiKey: 'p_JMpbuY216Pv0WCQFGijQLXwVIJzrf1dy55i3eCbNJDP', |
There was a problem hiding this comment.
This is a test private API key. Remember to put that in a secret before merging.
There was a problem hiding this comment.
@micheleriva does it use process.env keys? If so, can we reuse the ones from env variables we're already using?
We have these on Vercel.
There was a problem hiding this comment.
You can use these env variables, but we'll have to provide different keys right before we merge
There was a problem hiding this comment.
I'm fine with maybe slightly different keys then to prevent disruption. can we suffix all with _NEW ie: NEXT_PUBLIC_ORAMA_ENDPOINT_NEW?
There was a problem hiding this comment.
Why not duplicate the current ones to _OLD, change the env variables on main, and then put the new ones as the original variables?
There was a problem hiding this comment.
Open an issue then and ask @nodejs/web-infra to take care of it 🙇
| <Skeleton className={styles.searchButtonSkeleton} loading={true} /> | ||
| ), | ||
| }); | ||
| const SearchButton = dynamic( |
There was a problem hiding this comment.
Do we still need it to be a dynamic component? At the very elast can we flip ssr to true?
There was a problem hiding this comment.
Since the new components has "use client" and assuming the components are compatible, we probably don't need the dynamic at all no?
There was a problem hiding this comment.
@avivkeller shouldn't this SearchBox component be on the ui-components, or?
Description
Validation
Related Issues
Check List
pnpm formatto ensure the code follows the style guide.pnpm testto check if all tests are passing.pnpm buildto check if the website builds without errors.