plebbit / seedit

A GUI for plebbit similar to old.reddit
https://seedit.eth.limo/#/
GNU General Public License v2.0
13 stars 3 forks source link

add 'hidden' tab in profile page, listing all blocked addresses in account file #356

Open plebeius-eth opened 6 months ago

plebeius-eth commented 6 months ago

911fa767c5295bfdcc3b7d222352d1f9a2138609

estebanabaroa commented 5 months ago
const Profile = () => {
  const { t } = useTranslation();
  const account = useAccount();
  const location = useLocation();
  const params = useParams();
  let { accountComments } = useAccountComments();
  accountComments = [...accountComments].reverse();
  const { accountVotes } = useAccountVotes();
  const isInProfileUpvotedView = isProfileUpvotedView(location.pathname);
  const isInProfileDownvotedView = isProfileDownvotedView(location.pathname);
  const isInProfileHiddenView = isProfileHiddenView(location.pathname);
  const isInCommentsView = isProfileCommentsView(location.pathname);
  const isInSubmittedView = isProfileSubmittedView(location.pathname);
  const isMobile = useWindowWidth() < 640;

  // get comments for upvoted/downvoted/comments/submitted pages
  const postComments = useMemo(() => accountComments?.filter((comment) => !comment.parentCid) || [], [accountComments]);
  const replyComments = useMemo(() => accountComments?.filter((comment) => comment.parentCid) || [], [accountComments]);
  const upvotedCommentCids = useMemo(() => accountVotes?.filter((vote) => vote.vote === 1).map((vote) => vote.commentCid) || [], [accountVotes]);
  const downvotedCommentCids = useMemo(() => accountVotes?.filter((vote) => vote.vote === -1).map((vote) => vote.commentCid) || [], [accountVotes]);
  const hiddenCommentCids = useMemo(() => Object.keys(account?.blockedCids ?? {}), [account?.blockedCids]);

  const { comments: upvotedComments } = useComments({ commentCids: upvotedCommentCids });
  const { comments: downvotedComments } = useComments({ commentCids: downvotedCommentCids });
  const { comments: hiddenComments } = useComments({ commentCids: hiddenCommentCids });

this will destroy performance, every time the user goes to the profile page, even if he doesnt want to see his hidden posts, all the hidden posts will be loaded one by one, it will queue hundreds or thousands of ipfs gateway requests and the user won't be able to load anything else.

in general you should never use useComments with more than maybe 10 comments at a time. because it does 1 request for each comment, and they are super slow, and they clog up the maximum amount of ipfs gateway requests that the browser can do and it makes it impossible to load the rest of the app.

so what I suggest is to make a component <Upvoted> <Downvoted> <Hidden> that is only rendered when the user is on the correct route, and it should be paginated to only load 10 comments at a time (10 per page) using useComments, I wouldnt use infinite scroll, I would do profile/upvoted/page/2, you cannot pass all comments, there could be thousands, it will break the app to try to load all of them at once.

in general you should never use useComments, it has extremely bad performance. if you have to use it, like for example for the plebchan home page, or to load hidden/upvoted/downvoted, it should only be done with less than 10 comments at a time, and it should only be done if the user actually visits the page, it should never be done for no reason as it will render the web app unusable

estebanabaroa commented 5 months ago

also I think you can use <Routes> inside the view, I dont think you need to create your own routing components like isProfileUpvotedView(). I'm not 100% sure.

or it might just be easier to create a new view for hidden, upvoted and downvoted. the code would be mostly duplicated but I don't think it matters much if you're just importing a few reusable components from components/

plebeius-eth commented 1 month ago

refactored 9434a175397c9b442a2b38089ed5852a15770736

estebanabaroa commented 1 month ago

what is that

const CheckRouteParams = () => {
  const { accountCommentIndex, sortType, timeFilterName } = useParams();
  const isValidAccountCommentIndex = !accountCommentIndex || (!isNaN(parseInt(accountCommentIndex)) && parseInt(accountCommentIndex) >= 0);
  const isSortTypeValid = !sortType || sortTypes.includes(sortType);
  const isTimeFilterNameValid = !timeFilterName || timeFilterNames.includes(timeFilterName as any);
  const isAccountCommentIndexValid = !accountCommentIndex || !isNaN(parseInt(accountCommentIndex));

  if (!isValidAccountCommentIndex || !isSortTypeValid || !isTimeFilterNameValid || !isAccountCommentIndexValid) {
    return <NotFound />;
  }

  return <Outlet />;
};

why cant it use react-router normally?

you shouldnt code routing manually, you should use the library as its meant to be used, unless it's missing some feature I dont know about

              <Route path='/profile' element={<Profile />}>
                <Route index element={<Profile.Overview />} />
                <Route path='upvoted' element={<Profile.VotedComments voteType={1} />} />
                <Route path='downvoted' element={<Profile.VotedComments voteType={-1} />} />
                <Route path='hidden' element={<Profile.HiddenComments />} />
                <Route path='comments' element={<Profile.Comments />} />
                <Route path='submitted' element={<Profile.Submitted />} />
              </Route>

why is this code in app.tsx? it should be in views/profile.tsx

views should only export 1 file, the view, it shouldnt export individual components

you can use inside views, you dont have to do all the routing inside app.tsx

profile.tsx can do its own subrouting using

plebeius-eth commented 3 weeks ago

why cant it use react-router normally?

I don't want to use or something to redirect to , because I want the NotFound view to get routed both with a wildcard and with the custom hook. The hook useValidateRouteParams (prev. CheckRouteParams) checks dynamic params, for example: if a user has posted 3 times only, profile/1 will go to Profile, profile/100 will go to NotFound

I don't know if there's a better way to do this. I agree the hook looked confusing so I moved it in the hooks directory.

f2467cb4303c00fae00c4cb8525a24b5836ce052

plebeius-eth commented 3 weeks ago

it should be in views/profile.tsx

it can't, because we use profile/:accountCommentIndex as route for the pending post page, which has to be validated by useValidateRouteParams hook to prevent the user from manually going to an invalid accountCommentIndex, redirecting him to NotFound instead. This is why we can't use a profile/* wildcard in app.tsx, which would be needed if we were to move all the profile page routing to profile.tsx.

estebanabaroa commented 2 weeks ago

this can't be done imo:

const ValidatedRoute = () => {
  const isValid = useValidateRouteParams();

  if (!isValid) {
    return <NotFound />;
  }

  return <Outlet />;
};
<Route element={<ValidatedRoute />}>

if we want to display an error when the profile page has no posts, we should just display an error from inside the profile page. it's not necessary to do anything weird to the routing. also the user will almost never land on profile/<post doesnt exist> so we shouldnt make the code uglier and potentially buggy/less performant just to handle this use case that almost never happens.

we shouldnt implement our own routing ever imo, there's a reason we're using a routing library, it's clean, it's documented, it's self explanatory to people who read the code, it won't cause bugs, it wont have random performance issues and it wont be difficult to edit later.

adding random pieces of routing for no valid reason (the error can be handled in profile page) and using react-router in an undocumented and weird way is really bad, I am confused by that code and I wouldnt be able to add new functionality to it without being confused. I'm also not sure using react-router this way wont cause performance issues, we dont really know how many times this thing is rerendering and why/when. routing is special and low level, it has to be extremely fast cause it's done over and over and it's done before anything else happens.

estebanabaroa commented 2 weeks ago

it can't, because we use profile/:accountCommentIndex as route for the pending post page, which has to be validated by useValidateRouteParams hook to prevent the user from manually going to an invalid accountCommentIndex, redirecting him to NotFound instead. This is why we can't use a profile/* wildcard in app.tsx, which would be needed if we were to move all the profile page routing to profile.tsx.

imo we have to remove useValidateRouteParams and display a custom error in the profile page. maybe it would be slightly better UX to display the regular "not found" page, but it's an error that happens so rarely that imo it's not worth it to make the routing messy and confusing just for that.

also you could argue that displaying a custom error for profile/<invalid post> might be better. like we could figure out how the user ever landed on this page, and either redirect him to the correct page, or tell him specifically what he did wrong. even if that's not a 1:1 copy of reddit, it doesnt really matter cause it's something that almost never happens, like someone could use seedit for years and never see this error, so we're not really losing immersion.

the rule of thumb imo is we want to be almost 1:1 reddit UI for the core experience, stuff that people do over and over thousands of time, like the feed and post page, but for stuff that people do 0.00001% of the time, we don't really have to be 1:1, it doesn't matter that much.

estebanabaroa commented 2 weeks ago

also another way we could do it is:

  if (isNotFound) {
    return <NotFound />
  }

  return (
    <div className={`${styles.app} ${theme}`}>
      <Routes>
        // ....

this way we don't have to make the Profile page export components, views should just export 1 view (1 component) as a convention, if we start doing stuff randomly, it becomes really messy.

also I dont think we should do that, cause that would be a form of implementing our own routing, and we have to wait for useAccountComments() to calculate every time, and it might cause rerenders, etc. imo it's not worth it just to be able to send the NotFound component in this really rare scenario.