-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[APP-1031] Add new followerRule to threadgate settings #7681
[APP-1031] Add new followerRule to threadgate settings #7681
Conversation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, works perfectly. Filed #7692 separately.
My only suggestion is maybe disable this enormous CTA when the threadgate is enabled? What do you think? It's nbd to keep it if you disagree
![Screenshot 2025-02-07 at 22 51 08](https://private-user-images.githubusercontent.com/10959775/411109496-fff2d40a-d4d2-41c7-b684-285cc571d7d6.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzk0NTU4NDgsIm5iZiI6MTczOTQ1NTU0OCwicGF0aCI6Ii8xMDk1OTc3NS80MTExMDk0OTYtZmZmMmQ0MGEtZDRkMi00MWM3LWI2ODQtMjg1Y2M1NzFkN2Q2LnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTAyMTMlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwMjEzVDE0MDU0OFomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTg2YjBkNDIyN2QxZDZjODBiZTI1MGQzOGFkNmFhNmIzZDBkODg3ZTI0MjRmNDUyNTU2NzZkZDg2NmMyMWRmZmUmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.BbiipVUN-nHrlGwMKIJkYTAyu-71o0T7IzbFPeVWKss)
Yeah actually let's remove that in that case |
This reverts commit bc454da.
Discussed in Slack and decided to err on the side of increasing friction for following a user and then replying, if indeed the @mozzius noting that since we show placeholder data from the cache here and get the I felt this was better than the alternative of not showing the Follow button and then showing it once the thread fully loaded, since that will affect the vast majority of posts, and the small regression here only affects posts with this threadgate enabled. |
This reverts commit cadc46d.
Also noting that initially I had reverted the profile shadow use that was in place to ensure the reply button responded to a follow from the post thread. But I reverted that again so that post -> profile -> follow -> back to post enables replies. I think this would be confusing otherwise. |
Sorry for jumping in here with one small point that just occurred to me: if you view a post that has the Edit: I guess this would only be a concern where someone sees that the comment icon is greyed out or taps on it and nothing happens, and then decides to try following the author directly from the feed to see if that lets them comment, rather than opening the post. So... maybe a rather niche scenario I'm imagining, actually 😅 |
@surfdude29 no that's a good shout, maybe we should handle that case too. That also implies it should be hidden on the profile hover on web... I might need to double check other cases. It's never simple, is it 😅 |
I don't mind it being present on the hover popover etc - I think that's fine. the big CTA was enough |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Button flash is fine, not too bad
Adds
followersRule
support to interaction settings. App view and dataplane are already deployed and working, so all that's left is the UI.