Add slides link #267
Add slides link #267
Conversation
isDateFuture is already being called to conditionally render the block of code. It seems unnecessary to have another isDateFuture check inside the block as it will always resolve to true as long as the block is rendered.
WalkthroughThis update refines the frontend components in the Nuxt project to improve responsiveness and layout. In the session and events list components, styling adjustments include updated flex layouts, responsive image and text sizes, and the addition of a conditionally rendered NuxtLink for viewing session slides when a deck is present. The single event component now uses a more robust date handling mechanism by creating a valid Date object as a fallback and applies a simplified static class binding for status display. Additionally, the session detail type is extended by an optional deck property. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant C as Session/Events Component
participant L as NuxtLink (Deck Link)
participant P as Slides Page
U->>C: Load session list
C->>C: Check if session has deck
alt Deck exists
C->>L: Render deck link
U->>L: Click deck link
L->>P: Navigate to slides
else No deck
C->>U: Display session without slide link
end
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🪧 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: 0
🧹 Nitpick comments (1)
packages/frontendmu-nuxt/components/meetup/SessionList.vue (1)
52-54: Consider explaining the use of !m-0The addition of
!m-0to the link's class is using the important flag to override margins. While this works, it might be better to identify and address the source of the margin conflict for a more maintainable solution.Consider identifying the source of the margin issue and addressing it directly rather than using the
!importantflag (!m-0).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/frontendmu-nuxt/components/meetup/SessionList.vue(1 hunks)packages/frontendmu-nuxt/components/meetup/Single.vue(1 hunks)packages/frontendmu-nuxt/components/speaker/EventsList.vue(1 hunks)packages/frontendmu-nuxt/utils/types.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test
- GitHub Check: Cloudflare Pages: frontend-mu-nuxt
🔇 Additional comments (9)
packages/frontendmu-nuxt/utils/types.ts (1)
258-258: Good addition of the optional deck propertyAdding the optional
deckproperty to theSessionDetailinterface is a clean way to support linking to presentation slides. The change is minimal and non-breaking for existing code.packages/frontendmu-nuxt/components/speaker/EventsList.vue (3)
15-17: LGTM! Improved layout structureThe addition of
class="relative"to the list item establishes a positioning context for absolute elements within it, which is necessary for the overlay link implementation.
28-37: Well-implemented slides linkThe conditional rendering of the slides link is implemented well. The link opens in a new tab with appropriate security attributes (
rel="noopener noreferrer nofollow"). The styling creates a visually distinct clickable element with appropriate accessibility considerations.
40-40: Good UX improvement with overlay linkUsing an absolute overlay link is a good UX pattern - it makes the entire list item clickable while still allowing for distinct interactive elements inside (like the slides link).
packages/frontendmu-nuxt/components/meetup/Single.vue (2)
44-44: Improved date handlingThe change from
getCurrentEvent.Date || ''tonew Date(getCurrentEvent.Date || new Date)is a good improvement. It ensures that a valid Date object is always passed toisDateInFuture()rather than potentially an empty string.
48-48: Static class binding simplificationYou've simplified the class binding by using static classes rather than conditional ones. While this simplifies the code, ensure this doesn't remove any needed conditional styling based on the event's date status.
Could you confirm if there was an intentional design decision to move away from conditional styling based on the event date?
packages/frontendmu-nuxt/components/meetup/SessionList.vue (3)
25-31: Good responsive layout improvementsThe modifications to the layout structure and image sizes improve the responsiveness across different viewport sizes. Using responsive class utilities (
sm:,md:,lg:prefixes) provides a better mobile-first approach.
34-39: Improved typography for better responsivenessThe typography changes with responsive text sizes enhance readability across different devices, which is a good improvement.
40-49: Well-implemented slides linkThe conditional rendering for the slides link matches the implementation in EventsList.vue, maintaining consistency across components. Good use of accessibility attributes and styling.
n-d-r-d-g
force-pushed
the
feat/add-slides-link
branch
from
8631f35 to
a5acd22
Compare
Deploying frontend-mu-nuxt with
|
| Latest commit: |
a5acd22
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://16ea44ee.frontend-mu-staging.pages.dev |
| Branch Preview URL: | https://feat-add-slides-link.frontend-mu-staging.pages.dev |
|
Thanks @n-d-r-d-g ! i tested and it seems to work perfectly when slides are present ! I will merge this but i do have one small suggestions: let's use an icon instead of the hand emoji. You can pick something from icones.js.org and use it easily as we already have nuxt-icons setup in the repo. Would be really cool if you can send another PR to change that. |
This PR adds a link to access a session's slides whenever it's available.
Affected pages:
meetup/[id]speaker/[id]P.s. I noticed there was an open issue that seems to have been stale for a while.
meetup/[id]:speaker/[id]:Closes #44.
Summary by CodeRabbit
New Features
Style
Bug Fixes
Chores