[APP] remove ui folder#343
Merged
Merged
Conversation
Reviewer's GuideRefactors the shared UI library into the new features/core/ui module, updates all consumers and asset paths to use it, tightens type safety on style tokens, and relocates auth reset-password modal and core error pages into feature-scoped locations while adjusting architecture rules and tooling accordingly. File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Important Review skippedToo many files! This PR contains 240 files, which is 90 over the limit of 150. To get a review, narrow the scope: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (20)
📒 Files selected for processing (240)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In
BaseSkeleton.logic.ts,opacity.value = withRepeat(...)runs on every render; consider moving this side effect into auseEffectto avoid repeatedly restarting the animation. - In
PlacesAutocomplete.logic.ts,LocationInfois redefined instead of imported from the shared trip-generation entities, which risks the types diverging over time; consider centralizing this interface in a shared domain module.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `BaseSkeleton.logic.ts`, `opacity.value = withRepeat(...)` runs on every render; consider moving this side effect into a `useEffect` to avoid repeatedly restarting the animation.
- In `PlacesAutocomplete.logic.ts`, `LocationInfo` is redefined instead of imported from the shared trip-generation entities, which risks the types diverging over time; consider centralizing this interface in a shared domain module.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 258 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="features/core/ui/components/basic/BaseSkeleton/BaseSkeleton.logic.ts">
<violation number="1" location="features/core/ui/components/basic/BaseSkeleton/BaseSkeleton.logic.ts:6">
P2: Starting the repeating Reanimated animation in the hook body causes it to be re-triggered on each render; for this non-memoized component, parent/prop updates can reset the pulse animation and create unnecessary work.
(Based on your team's feedback about only flagging shared-value assignments when re-renders are possible.) [FEEDBACK_USED]</violation>
</file>
<file name="features/core/ui/components/basic/BaseSkeleton/BaseSkeleton.tsx">
<violation number="1" location="features/core/ui/components/basic/BaseSkeleton/BaseSkeleton.tsx:7">
P2: `children` is typed too narrowly as `ReactElement`, which can reject valid `View` child patterns. Use a `ReactNode`-compatible type instead.</violation>
</file>
Note: This PR contains a large number of files. cubic only reviews up to 100 files per PR, so some files may not have been reviewed. cubic prioritizes the most important files to review.
On a pro plan you can use ultrareview for larger PRs.
Tip: cubic used a learning from your PR history. Let your coding agent read cubic learnings directly with the cubic MCP.
Re-trigger cubic
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request executes a major architectural refactoring by migrating the global ui/ directory into a centralized features/core/ui/ module. The changes include updating asset paths in app.json, adjusting architecture and dependency rules, and refactoring imports across the entire project to utilize the new UI feature entry point. Feedback highlights a duplication of the LocationInfo interface, recommending its relocation to a shared domain or core directory to ensure a single source of truth.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #342