Save develop polish #970
Save develop polish #970
Conversation
EVENTS. Télécharger les participants
) Re 2782 WIP create events (#961)
WalkthroughThe pull request introduces a new DOMComponent within the VimeoPlayer for rendering iframes and revises several UI components. Dropdown, Input, and Select components have updated styling, theming, and layout logic. Event form components now conditionally render fields based on an isAuthor flag, and media rendering in event detail pages uses the VimeoPlayer when event conditions are met. Additionally, the event schemas have been extended to include a live_url field for streaming events. Changes
Sequence Diagram(s)sequenceDiagram
participant E as EventDetailComponent
participant V as VimeoPlayer
participant F as Fallback Image
E->>E: Evaluate if event.full is true\nand live_url starts with "https://vimeo.com/event/"
alt Valid Vimeo URL
E->>V: Render VimeoPlayer with live_url & DOM props
else
E->>F: Render fallback image
end
sequenceDiagram
participant EF as EventForm
participant Ctx as FormContext (isAuthor)
participant S as Scope Field
EF->>Ctx: Retrieve isAuthor flag
alt isAuthor is true
Ctx->>S: Render scope selection component
else
Ctx-->>EF: Omit scope selection component
end
Poem
Tip 🌐 Web search-backed reviews and chat
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
src/features/events/components/EventForm/ViewportModal.tsx (2)
24-27: Extract magic numbers into named constants and add min dimensions.While the calculations are reasonable, consider these improvements:
- Extract magic numbers into named constants for better maintainability
- Add minimum dimensions to ensure usability on small screens
+const MAX_WIDTH_PERCENTAGE = 80 +const MAX_WIDTH_PIXELS = 1048 +const HEIGHT_PERCENTAGE = 60 +const MIN_WIDTH = 320 +const MIN_HEIGHT = 240 const size = useWindowDimensions() -const width = Math.min((size.width * 80) / 100, 1048) -const height = (size.height * 60) / 100 +const width = Math.max(MIN_WIDTH, Math.min((size.width * MAX_WIDTH_PERCENTAGE) / 100, MAX_WIDTH_PIXELS)) +const height = Math.max(MIN_HEIGHT, (size.height * HEIGHT_PERCENTAGE) / 100)
34-34: Extract Sheet snapPoints magic number into a constant.For consistency with the other dimension calculations, consider extracting the Sheet's snapPoints value into a named constant.
+const SHEET_SNAP_POINT_PERCENTAGE = 80 <Sheet modal open={!!open} - snapPoints={[80]} + snapPoints={[SHEET_SNAP_POINT_PERCENTAGE]} snapPointsMode="percent" // ... other props >Also applies to: 48-48
src/components/VimeoPlayer.tsx (1)
3-5: Enhance accessibility and configurability.Consider the following improvements:
- Add accessibility attributes to the iframe.
- Make the height configurable via props.
Apply this diff to implement the suggestions:
-export default function DOMComponent(props: { url: string; dom: import('expo/dom').DOMProps }) { - return <iframe width="100%" height="315" src={props.url} frameBorder="0" allow="autoplay; fullscreen; picture-in-picture" allowFullScreen></iframe> +type DOMComponentProps = { + url: string; + dom: import('expo/dom').DOMProps; + height?: number; + title?: string; +}; + +export default function DOMComponent({ url, dom, height = 315, title }: DOMComponentProps) { + return ( + <iframe + width="100%" + height={height} + src={url} + frameBorder="0" + allow="autoplay; fullscreen; picture-in-picture" + allowFullScreen + title={title ?? 'Vimeo video player'} + aria-label={title ?? 'Vimeo video player'} + /> + ); +}src/components/base/Select/SelectV3.tsx (1)
69-69: Use optional chaining for safer property access.Consider using optional chaining to safely access the
subLabelproperty.Apply this diff to implement optional chaining:
- {fullValue && fullValue.subLabel && props.showSubLabel ? <SF.Text fontSize={12}>{fullValue.subLabel}</SF.Text> : null} + {fullValue?.subLabel && props.showSubLabel ? <SF.Text fontSize={12}>{fullValue.subLabel}</SF.Text> : null}🧰 Tools
🪛 Biome (1.9.4)
[error] 69-69: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/features/events/pages/detail/EventMobileScreen.tsx (1)
95-104: Enhance URL validation for Vimeo links.While the current implementation works, the URL validation could be more robust.
Consider using a more specific regex pattern:
- {isFull && event.live_url && event.live_url.startsWith('https://vimeo.com/event/') ? ( + {isFull && event.live_url && /^https:\/\/vimeo\.com\/event\/\d+(?:\/[a-f0-9]+)?$/.test(event.live_url) ? (src/features/events/components/EventForm/EventFormDesktopScreen.tsx (1)
62-62: Fix typo in label text.The label text "Accèes" should be "Accès" in French.
- label="Accèes" + label="Accès"src/features/events/components/EventForm/EventFormMobileScreen.tsx (1)
187-187: Fix typo in label text.The label text "Accèes" should be "Accès" in French.
- label="Accèes" + label="Accès"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
src/components/VimeoPlayer.tsx(1 hunks)src/components/base/Dropdown/index.tsx(4 hunks)src/components/base/Input/Input.tsx(3 hunks)src/components/base/Select/SelectBottomSheet.tsx(1 hunks)src/components/base/Select/SelectV3.tsx(1 hunks)src/components/base/Select/types.ts(1 hunks)src/components/base/Select/useSelectSearch.tsx(1 hunks)src/features/events/components/EventForm/DescriptionInput.tsx(2 hunks)src/features/events/components/EventForm/EventFormDesktopScreen.tsx(2 hunks)src/features/events/components/EventForm/EventFormMobileScreen.tsx(4 hunks)src/features/events/components/EventForm/ViewportModal.tsx(2 hunks)src/features/events/components/EventForm/context.tsx(5 hunks)src/features/events/pages/detail/EventDesktopScreen.tsx(2 hunks)src/features/events/pages/detail/EventMobileScreen.tsx(2 hunks)src/services/events/schema.ts(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/components/base/Select/SelectV3.tsx
[error] 69-69: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (25)
src/features/events/components/EventForm/ViewportModal.tsx (1)
5-5: LGTM! Good use of the useWindowDimensions hook.The import is correctly grouped with other tamagui imports and will help with responsive design.
src/components/VimeoPlayer.tsx (1)
3-3: Verify the type import.The type import for
DOMPropsis using a dynamic import statement. Consider importing it directly if the type is available.src/components/base/Select/useSelectSearch.tsx (1)
15-15: LGTM!The addition of
themeandiconproperties to the mapped items is consistent with theSelectOptiontype and enhances the component's theming capabilities.src/components/base/Select/types.ts (1)
33-33: LGTM!The addition of the optional
showSubLabelprop enhances the component's flexibility by allowing control over sub-label visibility.src/components/base/Select/SelectV3.tsx (1)
66-68: LGTM!The text component changes enhance visual feedback by using the
secondaryprop when no value is selected.src/components/base/Select/SelectBottomSheet.tsx (2)
70-70: LGTM! Consistent with Dropdown changes.The
size={false}prop aligns with the new variant in DropdownFrame that allows for an unset maxHeight.
74-74: LGTM! More explicit keyboard behavior.Using
"always"instead of a boolean value makes the keyboard persistence behavior more explicit.src/components/base/Dropdown/index.tsx (2)
8-8: LGTM! Enhanced theming support.The integration with SF components and theme-based rendering improves the component's flexibility and customization options.
Also applies to: 65-82
111-113: LGTM! Added flexibility for height control.The new
falsesize variant allowingmaxHeight: 'unset'provides better control over dropdown dimensions.src/services/events/schema.ts (1)
79-79: LGTM! Consistent schema updates.The
live_urlfield is appropriately added as nullable/optional across all relevant schemas and property paths.Also applies to: 221-221, 243-243
src/features/events/components/EventForm/DescriptionInput.tsx (2)
7-7: LGTM!The addition of the
Saveicon from@tamagui/lucide-iconsaligns with the UI enhancement for the save action.
242-258: LGTM!The header layout has been improved by:
- Simplifying the UI to focus on the save action
- Adding the
Saveicon to provide visual feedback- Using proper flex properties for alignment
src/components/base/Input/Input.tsx (4)
60-60: LGTM!The hover style now uses the theme-based color
$gray2, improving theme consistency.
110-141: LGTM!The size variants and multiline support have been well-implemented with:
- Consistent border radius progression
- Proper height scaling
- Flexible multiline handling with auto height and minimum height constraints
244-245: LGTM!The size prop is now correctly handled based on the multiline state.
254-254: LGTM!The YStack layout has been improved with proper height and padding for multiline inputs.
src/features/events/components/EventForm/context.tsx (4)
8-8: LGTM!Added
useGetSuspenseProfilhook for profile data retrieval.
99-99: LGTM!The scope options are now correctly filtered to include only scopes with the 'events' feature.
50-76: LGTM!The visibility options have been enhanced with clear and detailed descriptions that help users understand the implications of each option.
103-108: LGTM!The
isAuthorcheck is well-implemented with proper null checks and comparison logic.src/features/events/pages/detail/EventDesktopScreen.tsx (2)
5-5: LGTM!Added VimeoPlayer component import for video content support.
111-120: LGTM!The media rendering logic has been enhanced to:
- Support Vimeo event videos with proper URL validation
- Maintain the existing fallback image functionality
src/features/events/components/EventForm/EventFormDesktopScreen.tsx (1)
26-26: LGTM! Conditional rendering of scope selector based on author status.The implementation correctly restricts scope selection to authors only, improving security and user experience.
Also applies to: 31-53
src/features/events/components/EventForm/EventFormMobileScreen.tsx (2)
93-93: LGTM! Consistent implementation of author-based scope selection.The mobile implementation correctly mirrors the desktop version, maintaining a consistent user experience across devices.
Also applies to: 137-159
206-206: LGTM! Improved UX with placeholder text.The addition of
noValuePlaceholderfor the category selector enhances user experience by providing clear guidance.
| }, | ||
| }, | ||
| defaultVariants: { | ||
| size: 'md', | ||
| size: 'xl', |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify the impact of changing default size.
The default size variant has been changed from 'md' to 'xl', which could affect existing dropdown instances.
🏁 Script executed:
#!/bin/bash
# Search for Dropdown usage without explicit size prop
rg -A 5 '<Dropdown(?![^>]*size=)'Length of output: 271
🏁 Script executed:
#!/bin/bash
# Re-run the search using PCRE2 for lookahead support to find <Dropdown usages without an explicit size prop
rg --pcre2 -A 5 '<Dropdown(?![^>]*\bsize=)'Length of output: 2480
Default Size Change May Impact Dropdown Instances
Our investigation confirms that several instances of the Dropdown component (or its variants) are used without an explicit size property. For example:
- In src/components/base/Dropdown/index.tsx, the Dropdown is invoked without specifying a size, meaning it now uses the new default ('xl') instead of the former 'md'.
- Similar patterns were observed in related components (e.g., the DropdownFrame in the SelectDropdown and DropdownWrapper in ProfilBlock).
This change will indeed affect existing dropdown instances by applying a larger default size, which may lead to unintended layout or design issues. It would be advisable to review the impacted components and update them with an explicit size prop if the previous 'md' behavior is required.
OverGlass
force-pushed
the
develop
branch
2 times, most recently
from
3850ec7 to
0ff6df1
Compare
Summary by CodeRabbit