Subscribed tab feed improvements 20240514 #9289
Conversation
| @@ -28,15 +31,23 @@ const styles = (_theme: ThemeType) => ({ | |||
| */ | |||
| const UserNotifyDropdown = ({ | |||
| user, | |||
| isFriendlyUI=true, | |||
There was a problem hiding this comment.
| isFriendlyUI=true, |
There was a problem hiding this comment.
This isn't the idomatic way to do this.
| classes, | ||
| }: { | ||
| user: UsersProfile, | ||
| isFriendlyUI?: boolean, |
There was a problem hiding this comment.
| isFriendlyUI?: boolean, |
| @@ -1,6 +1,9 @@ | |||
| import React, { useCallback, useRef, useState } from 'react'; | |||
| import { Components, registerComponent } from '../../lib/vulcan-lib'; | |||
| import { useTracking } from '../../lib/analyticsEvents'; | |||
| import { userHasSubscribeTabFeed } from '../../lib/betas'; | |||
| import { useCurrentUser } from '../common/withUser'; | |||
| import { PopperPlacementType } from '@material-ui/core/Popper/Popper'; | |||
There was a problem hiding this comment.
| import { PopperPlacementType } from '@material-ui/core/Popper/Popper'; | |
| import { PopperPlacementType } from '@material-ui/core/Popper/Popper'; | |
| import { isFriendlyUI } from '../../themes/forumTheme'; |
|
I'm getting a flash of a loading spinner where I didn't before on our |
This is the element I most wanted feedback on. When I first tried the component, I got a loading spinner even though I wasn't seeing one when using the component live on EA Forum. My changes were to try to make the loading spinner look a little better than how it was before. I don't think I did anything to change whether or not there is a loading spinner. Maybe it's just faster in prod so it doesn't show there? |
| ); | ||
|
|
||
| return ( | ||
| <DropdownItem | ||
| title={title} | ||
| onClick={toggleSubscribed} | ||
| afterIcon={afterIcon} | ||
| loading={loading} |
There was a problem hiding this comment.
This could be the loading spinner change
There was a problem hiding this comment.
Ah, makes sense. So you say above "getting a flash of loading spinner" – do you want me to forum gate changes that cause this?
There was a problem hiding this comment.
I think so? I haven't thought about what's going on too deeply, it could be that the loading spinner is "correct."
| @@ -51,12 +51,12 @@ const ContentItemTruncated = ({classes, maxLengthWords, graceWords=20, expanded= | |||
| </> | |||
| } | |||
|
|
|||
| const truncateWithGrace = (html: string, maxLengthWords: number, graceWords: number, rawWordCount: number): { | |||
| export const truncateWithGrace = (html: string, maxLengthWords: number, graceWords: number, rawWordCount: number, suffix?: string): { | |||
There was a problem hiding this comment.
Recommend moving this to the same file as truncate comes from.
| classes, | ||
| }: NotifyMeToggleDropdownItemInternalProps) => { | ||
| const {isSubscribed, onSubscribe} = useNotifyMe({ | ||
| const {isSubscribed, onSubscribe, loading, disabled } = useNotifyMe({ |
|
|
||
| return <ToggleSwitch value={subscribed} className={classes.toggle} smallVersion /> | ||
| }, | ||
| [subscribed, useCheckboxIcon, ToggleSwitch, classes.toggle ], |
| TruncatedAuthorsList, PostsItemDate, ForumIcon, LWTooltip, EventTime, FormatDate | ||
| } = Components; | ||
|
|
||
| // TODO: Think about styling for events |
There was a problem hiding this comment.
does this TODO need something?
| const authorsListElement = <InteractionWrapper className={classes.interactionWrapper}> | ||
| <TruncatedAuthorsList | ||
| post={post} | ||
| expandContainer={authorExpandContainer} | ||
| className={classes.authorsList} | ||
| /> | ||
| {separatorElement} | ||
| </InteractionWrapper> |
There was a problem hiding this comment.
something about the indentation here looks weird
|
|
||
| const styles = (theme: ThemeType) => ({ | ||
| root: { | ||
|
|
| root: { | ||
|
|
||
| '& .read-more-button': { | ||
| // color: theme.palette.primary.main, |
| marginBottom: 10, | ||
| }, | ||
| highlightContinue: { | ||
| marginTop:theme.spacing.unit*2, |
| expandedDocument?: PostsExpandedHighlight, | ||
| classes: ClassesType, | ||
| }) => { | ||
| const { htmlHighlight = "", wordCount = 0 } = post.contents || {}; |
packages/lesswrong/components/recentDiscussion/FeedPostCommentsCard.tsx
Outdated
Show resolved
Hide resolved
packages/lesswrong/components/recentDiscussion/FeedPostCommentsCard.tsx
Outdated
Show resolved
Hide resolved
packages/lesswrong/components/recentDiscussion/FeedPostCommentsCard.tsx
Outdated
Show resolved
Hide resolved
packages/lesswrong/components/subscriptions/SuggestedFeedSubscriptions.tsx
Outdated
Show resolved
Hide resolved
packages/lesswrong/components/recentDiscussion/FeedPostCommentsCard.tsx
Outdated
Show resolved
Hide resolved
@jpaddison3 @oetherington I'm making use of the
UserNotifyDropdownand have changed it in a few ways that plausibly affect EAF (maybe for the better). (1) I piped through existing loading and disabled props from useNotify, (2) I changed the loading spinner to encompass the full contents of the menu items because otherwise, it looked really janky. You might want to check out the branch to check everything is still how you like it