mattermost / i18n-wip

Work in progress language files for the https://github.com/mattermost/mattermost
Apache License 2.0
4 stars 15 forks source link

Clarify Add Member to Channel Hint Text #9

Closed cwarnermm closed 3 years ago

cwarnermm commented 3 years ago

When adding members to a channel, the existing hint text ambiguously suggests that a maximum of 20 members can be added to a given channel. Updated hint text to note that users can add up to 20 members at a time to the channel.

cwarnermm commented 3 years ago

@esethna - This PR is to address community feedback. See this conversation for context.

esethna commented 3 years ago

Thanks @cwarnermm! I'll leave it to devs to comment, but I think this also needs to be changed in the webapp repo? (I might be wrong if the process has changed)

cwarnermm commented 3 years ago

Thanks, @esethna. I'm confirming the process now.

matthewbirtch commented 3 years ago

@cwarnermm the number that's displayed appears to be the number of people remaining that you can invite via this modal. The copy change doesn't seem to imply a remaining number anymore?

I think the number you're using in the code here will change as you add people to the input field, which may be a little odd to say "You can add up to 20 people at a time", then to see that number counting down.

We should get an engineer to review this too to make sure.

cwarnermm commented 3 years ago

Thanks so much, @matthewbirtch. I agree with confirming the intent and function of this count with R&D is required since the message will need to change depending on how the code functions.

cwarnermm commented 3 years ago

@hmhealey - Can you confirm how the code that drives this in-product string works, please? Is the value of this string always "20", or does it dynamically change by counting up (based on the number of members that the user has added to the channel starting from 0?), or by counting down (based on the number of members the user has added to the channel, starting from 20?)

Depending on how the code functions, I may need to revise my string update.

hmhealey commented 3 years ago

It counts down as the user selects more users (the code for that is here).

But that component is used for more things than just adding channel members. like adding LDAP groups to teams/channels in the System Console. If we can't find a message that also supports those cases, we'd need code changes to override the string for the Add Member modals

cwarnermm commented 3 years ago

Excellent, thank you @hmhealey! I've updated my string proposal based on how this component works. The existing message has led some to think that a maximum of 20 users are permitted in a given channel. I've added context to clarify that a maximum of 20 entities can be added in the current operation by using the term "batch". Thoughts? CC @matthewbirtch

esethna commented 3 years ago

@cwarnermm maybe, "You can add {num, number} more people at a time"? Batch might be confusing terminology

cwarnermm commented 3 years ago

@esethna - This component is used in other contexts beyond this one, so we need to remain generic enough to cover all use cases, or introduce code logic.

esethna commented 3 years ago

Ah then, perhaps we just drop the word "people"?

cwarnermm commented 3 years ago

@matthewbirtch @hmhealey Any concerns about this approach from your perspectives?

matthewbirtch commented 3 years ago

@matthewbirtch @hmhealey Any concerns about this approach from your perspectives?

"You can add X more in this batch"

"You can add X more at a time"

I find both of this confusing. What we're trying to say is "20 users can be added at a time. You have X remaining", right? Not saying this is what we should go with, but I don't think either of the above covers that.

cwarnermm commented 3 years ago

@matthewbirtch You're absolutely right and maybe the straightforward way through is to use a string that says exactly what we're intending. I've updated the string proposal accordingly. CC @esethna

@hmhealey - I've added a {max} placeholder in the latest string draft rather than hardcoding the value to "20". From a code perspective, is my approach feasible/correct? If so, what would that string placeholder need to change to based on the code?

cwarnermm commented 3 years ago

Thanks, @hmhealey! Are any additional changes needed to support this string text update?

hmhealey commented 3 years ago

I'm not actually sure how this repo is supposed to work since I've never seen it before. Generally, we update en.json as part of webapp PRs since the English string also appears in the web app code, so I'm not sure how we'd line this up with web app changes.

metanerd commented 3 years ago

Please create a PR for the mattermost-webapp repo. https://github.com/mattermost/mattermost-webapp/blob/6491690321974d9b0f9db93ac13a359e1a2a0b06/i18n/en.json#L3593

cwarnermm commented 3 years ago

Thanks, @metanerd! I've confirmed the very same with the localization team. I'll get this in the correct location.