-
-
Notifications
You must be signed in to change notification settings - Fork 7k
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
Add configuration option to filter replies in lists #9205
Conversation
389ba55
to
cd98334
Compare
I kept messing up the condition 😩 UI-wise, it would probably be better to have a dropdown, but the only dropdown of this kind is the privacy dropdown, and it's not easily reusable. |
Would anyone actually make use of the “show only replies to users in the list” option? This seems to cover most—if not all—needs, and it would make the code and UI simpler if it were the only option. CC @cbayerlein @saper as you were the main people discussing this in #5916 |
I think it'sfine as it is now, although it's always good to provide options to the users. |
I would use those 3 and also a "show all replies" option, ideally. It depends on the list. I'd conceptualize the setting as the following:
edit for clarification: this is in reply to @ThibG:
|
@trwnh can you be more specific?
…On Thu, Nov 8, 2018, 10:58 AM trwnh ***@***.***> wrote:
I would use those 3 and also a "show all replies" option, ideally. It
depends on the list.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#9205 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAORVyRvA9ffxOR9LfLcyTeqCQbuFJ4iks5utFSigaJpZM4YNVo7>
.
|
regardless, I absolutely agree with gargron that we shouldn't be doing this
filtering server side
…On Thu, Nov 8, 2018, 10:58 AM trwnh ***@***.***> wrote:
I would use those 3 and also a "show all replies" option, ideally. It
depends on the list.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#9205 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAORVyRvA9ffxOR9LfLcyTeqCQbuFJ4iks5utFSigaJpZM4YNVo7>
.
|
Why? (Also, uh, it would be nice if said opinion of gargron was actually in the issue?) |
I would personnaly like to use both the "Show only replies to users in the list" and "Show all replies to followed users" options. |
6f869fb
to
fb7fdf1
Compare
Got some feedback from @nightpool on discord (I'll paraphrase them, and quotes will be from them):
I personally think there is value in having the choices I described, and I think there is value in doing the filtering server-side (having the same behavior across clients without needing special support, avoiding useless data traffic), but I also understand the point about the filter not applying retroactively being confusing. |
fb7fdf1
to
1b85c7f
Compare
1b85c7f
to
fe02e79
Compare
About the filtering not being retroactive, the same can be said about list creation itself: only toots received after the list has been created will appear there. If you think changing filtering after creation is too confusing, it could be possible to choose the type of filtering upfront and not providing any UI to change it (although I would find it problematic). Of course, we could also not filter at all in the backend and do it in the front-end instead, but this may lead to inconsistent behavior across clients if they are not updated to deal with a new API, and it will also cause increased traffic, as unneeded toots may be streamed for every list. I'm open to implementing client-side filtering, although I prefer server-side filtering for the above reasons (and also because I've already done the work, tbh). |
fe02e79
to
4d35b68
Compare
I don't really understand why this is not merged yet. The new list system makes no sense, what is the point of using lists if you also have to scroll your home to see if you don't miss anything? :s |
bb1f594
to
9185b94
Compare
@ThibG Looks good! I'd just ask for one more option for "everyone", which shows all replies that you have permission to see. |
@trwnh I will consider this, but as this would show more thing than the Home timeline we currently have, this seems a bit counter-intuitive. Also, this is slightly more involved as toots delivered to lists are pre-filtered with the same criteria as for the Home timeline. |
@ThibG I guess that's true. The reason I'm interested in it is because I miss the functionality from Tweetdeck circa 2011 where you could show all replies in both home timelines and in lists. That might be a separate request, though... I should probably look into a separate issue for that. |
@trwnh i think this can be added later, I would also like to have this option for both the home timeline and lists |
9185b94
to
e63af3b
Compare
e63af3b
to
8a7c453
Compare
I've personally been using a few lists, one of which is set to “any followed user” while the others are using the default “members of the list” option. It is extremely useful to me, and I've seen multiple people ask for something like this. Can this PR be reconsidered? Would changing it to do the filtering client-side help? |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
I think this PR is still relevant. Without replies in lists users need to scroll the home timeline (miss the point of using lists) or peek at profile pages (not viable with large lists) |
Reopening, but I doubt Gargron changed his mind on this :/ |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Bump |
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.
LGTM but wondering about API naming
safety_assured do | ||
add_column_with_default( | ||
:lists, | ||
:replies_policy, |
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.
This is the first enum in our code we call _policy
, I don't want to bikeshed too much but is this a fully appropriate name?
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.
This has been in glitch-soc for almost two years, so I'd need to check if any client uses that API, but otherwise I'm fine with changing the name if you have a better suggestion.
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.
Actually upon seeing the screenshot down thread I have a few more concerns
<legend><FormattedMessage id='lists.replies_policy.title' defaultMessage='Show replies to:' /></legend> | ||
{ ['no_replies', 'list_replies', 'all_replies'].map(policy => ( | ||
<div className='setting-radio'> | ||
<input className='setting-radio__input' id={['setting', 'radio', id, policy].join('-')} type='radio' value={policy} checked={replies_policy === policy} onChange={this.handleRepliesPolicyChange} /> |
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.
Plain radio buttons look rather unsightly in there, can we use the toggles we use in other column settings?
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.
Someone pointed out that we already have radio buttons somewhere else in the interface (directory settings), I'll have a look to reuse that styling.
8a7c453
to
b428c0e
Compare
b428c0e
to
aa19de1
Compare
aa19de1
to
79d70fb
Compare
* Add database support for list show-reply preferences * Add backend support to read and update list-specific show_replies settings * Add basic UI to set list replies setting * Add specs for list replies policy * Switch "cycling" reply policy link to a set of radio inputs * Capitalize replies_policy strings * Change radio button design to be consistent with that of the directory explorer
* Add database support for list show-reply preferences * Add backend support to read and update list-specific show_replies settings * Add basic UI to set list replies setting * Add specs for list replies policy * Switch "cycling" reply policy link to a set of radio inputs * Capitalize replies_policy strings * Change radio button design to be consistent with that of the directory explorer
There are three possible settings:
Changing is done by
clicking on the link, which cycles between the settings (yeah, that's not great UI)a set of radio buttons.Note that only “show all replies” shows mentions to yourself. That could probably be changed.All settings show replies to yourself.