Skip to content

Custom author name not reflecting in summary #3415

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

Closed
gupta-anmol opened this issue Feb 15, 2020 · 13 comments
Closed

Custom author name not reflecting in summary #3415

gupta-anmol opened this issue Feb 15, 2020 · 13 comments

Comments

@gupta-anmol
Copy link
Contributor

Summary:

Custom author name preference should reflect the text value change in the summary, instead of repeating the information in the summary of Use custom author name switch preference.

Steps to reproduce:

How can we reproduce the issue?

Go to Settings and enable the Use custom author name. Change the name by tapping on the Custom author name preference.

What did you expect the app to do, and what did you see instead?

I expected the current custom name to be reflected in the summary of that preference. However, the summary is fixed redundant information. I say redundant because we have already conveyed the meaning of the preference to the user.

Device and Android version:

Redmi Note 5 Pro
MIUI version 11
Android 9

Commons app version:

master and betaDebug

Screen-shots:

Changing Custom author name preference

image

After tapping OK

image

Would you like to work on the issue?

Yes. If we can agree that the issue is genuine, then I'd love to start working on it.

@maskaravivek
Copy link
Member

Yes, I like the idea of displaying the author's name instead of repeating the text. Also, let us merge both these settings ie "Use custom author name" and "Custom author name".

The behavior can be:

  • when "use custom author name" is switched enabled, let us show the dialog box where the user can edit the author name.
  • when the switch is disabled, don't use the custom author name.

@gupta-anmol
Copy link
Contributor Author

@maskaravivek Good idea. Can I start working on this?

@sivaraam
Copy link
Member

Also, let us merge both these settings ie "Use custom author name" and "Custom author name".

That's a good thing to do but ...

The behavior can be:

* when "use custom author name" is switched enabled, let us show the dialog box where the user can edit the author name.

* when the switch is disabled, don't use the custom author name.

... IMO, merging this the other way round would be more intuitive. By other way round I mean merging "Use custom author name" into "Custom author name". That way, we would only have the "Custom author name" option.

  • When a user has not set a custom author name, we show the description of the option in the summary.
  • When a user has set a custom author name, we show the author name that has been set in the summary.
  • Tapping on the option would open the dialog to enter a custom author name.

@maskaravivek
Copy link
Member

... IMO, merging this the other way round would be more intuitive. By other way round I mean merging "Use custom author name" into "Custom author name". That way, we would only have the "Custom author name" option.

The approach that I suggested would also have just 1 option ie "use custom author name". Maybe we can improve the copy text. With this approach, the user can simply toggle off the setting to stop using the custom author name instead of opening the dialog box, erasing the custom name and saving it. When the toggle is off, we can continue to store the custom author name is the shared prefs and the user can come back later and enable it again. He could choose to either edit it or continue using the previously set author name.

When a user has not set a custom author name, we show the description of the option in the summary.
When a user has set a custom author name, we show the author name that has been set in the summary.
Tapping on the option would open the dialog to enter a custom author name.

The issues I see with this approach are:

  • that to disable the custom author name, the user will have to erase the custom author name and save it.
  • our logic that checks and uses custom author names would change. It will have to start checking for empty values. On the other hand with the approach that I had proposed, it would just be a UI change and our key/value pairs in the shared prefs would continue to have the same meaning and behavior.

@sivaraam
Copy link
Member

The approach that I suggested would also have just 1 option ie "use custom author name".

Yeah, I understood that initially.

Maybe we can improve the copy text. With this approach, the user can simply toggle off the setting to stop using the custom author name instead of opening the dialog box, erasing the custom name and saving it. When the toggle is off, we can continue to store the custom author name is the shared prefs and the user can come back later and enable it again. He could choose to either edit it or continue using the previously set author name.

That sounds useful. The reason I found merging the options into a toggle option unintuitive is that it would make the toggle option do more than just toggling an option. That's why I was suggesting to merge the other way round. Alternatively, keeping both the options and just update the summary sounds good too.

When a user has not set a custom author name, we show the description of the option in the summary.
When a user has set a custom author name, we show the author name that has been set in the summary.
Tapping on the option would open the dialog to enter a custom author name.

The issues I see with this approach are:

* that to disable the custom author name, the user will have to erase the custom author name and save it.

Yeah. We could provide an option to easily clear the author name via an 'x' shown near the text field.

* our logic that checks and uses custom author names would change. It will have to start checking for empty values. On the other hand with the approach that I had proposed, it would just be a UI change and our key/value pairs in the shared prefs would continue to have the same meaning and behavior.

This sounds more like an implementation detail than like an issue to me. I couldn't understand why this would matter.

@maskaravivek
Copy link
Member

This sounds more like an implementation detail than like an issue to me. I couldn't understand why this would matter.

It would matter for people who have already installed the app, have the custom author name set but the toggle disabled. Once the app updated, their custom author name will start getting used which might not be what they wanted.

@sivaraam
Copy link
Member

This sounds more like an implementation detail than like an issue to me. I couldn't understand why this would matter.

It would matter for people who have already installed the app, have the custom author name set but the toggle disabled. Once the app updated, their custom author name will start getting used which might not be what they wanted.

Ah! That's an issue indeed. We can think of a migration plan to handle this. During the migration,

  • If the "Use custom author name" switch is turned-off, we clear the author name filled by the user (if any)
  • If the "Use custom author name" switch is turned-on, we just use the author name provided by the user

Just a suggestion 🙂

@maskaravivek
Copy link
Member

True, but UX wise I would prefer having a toggle instead of making the user clear the custom author name.

Let us get some more opinions and then a volunteer can pick the task.

@sivaraam
Copy link
Member

True, but UX wise I would prefer having a toggle instead of making the user clear the custom author name.

Yeah, disabling a toggle would indeed be better. I suppose we should just keep the two option as it is then. That would be a lot better, IMO.

Let us get some more opinions and then a volunteer can pick the task.

Sure.

@ashishkumar468
Copy link
Collaborator

Personally I think, a toggle would be simpler in terms of UX, disabling the author name by a switch surely seems better than making the user manually clear it.

@gupta-anmol
Copy link
Contributor Author

Yeah, disabling a toggle would indeed be better. I suppose we should just keep the two option as it is then. That would be a lot better, IMO.

I agree with @sivaraam, the toggle will perhaps become unintuitive if it would do more than just toggling. I have rarely seen the toggle doing more. For example, in the official Reddit app, there's an option to toggle the dark mode and a separate option to choose the dark mode theme. This is similar to our issue. I can imagine it getting slightly annoying if the app asked me my dark theme preference every time I turned on dark mode.

image

@gupta-anmol
Copy link
Contributor Author

@maskaravivek How should we proceed with this?

@sivaraam
Copy link
Member

sivaraam commented Mar 19, 2020

Fixed in #3537

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants