bug: Can't enter "null" in patch options

[Shane9248]

Bug description

In the patch 'Custom Branding' there should be an option to not change the icon, just like you can choose not to touch the app name

The only options are "ReVanced logo" and "Custom value"

Version of ReVanced Manager and version & name of application you tried to patch

Version - 1.18.0
Application - YouTube Revanced

Installation type

None

Device logs

.

Patcher logs

No response

Acknowledgements

• This request is not a duplicate of an existing issue.
• I have chosen an appropriate title.
• All requested information has been provided properly.
• The issue is solely related to the ReVanced Manager

[Shane9248]

ReVanced/revanced-patches#2762 (comment) as mentioned here i am now told ro redirect to here

[Ushie]

Doesn't deleting the option count as null?

@oSumAtrIX @Shane9248

[oSumAtrIX]

Deleting resets the option value to default. If the default is null, it will reset to null

[Ushie]

That sounds weird, if the default is a non-null value then the option by default is added in ReVanced Manager, removing it then should make it null

If the default is null, it should be manually added by the user

[oSumAtrIX]

This is not the case. If you haven't configured any options, the options are not added in this view. Instead you have to click on plus to add them and change them. The reason for this is to cover this scenaro:

An option value is null by default. You want to change it, so you add it. Now you want to set it back to null. As you can not input "null", you click on the icon to delete it which resets the value to null. This issue refers to the problem, when the option accepts null, but isn't null by default, so you have no way to set it to null.

To improve UX some changes can be made:

• Add all options to this view, get rid of this "Add option" feature
• Instead of the delete button, make it a "Reset" button
• In the custom field dropdown menu, add a "Set to null" option, if and only if option.required == false

This would fix the current issue and the slightly weird UX

[Ushie]

You're overcomplicating it, the issue here is that "Delete" and "Reset" are being used in a single button, simply adding a Reset button is enough

I don't agree with showing all options, if an option is null by default then it shouldn't be added by default, that introduces some weird UX, especially accompanied with your "Set to null" option which is a glorified delete button (that you want to replace with reset)

[oSumAtrIX]

I'd like an elaboration on what is being complicated here. From what I am seeing, my suggestion simplifies the usage.

simply adding a Reset button is enough

No, as the current delete functionality is to reset. By adding a reset button, you try repurposing this as a way to set it to null, whereas the dropdown menu is the correct purpose for abstract inputs such as paths to folders files, or null in this case.

[Ushie]

Null is just the computer way of saying nothing, a Delete button makes something nothing.

The issue here is that the delete button isn't a delete button, it's a reset button, a proper delete button should be added and the existing button should have the icon updated to fix the weird UX and impossibility of manually setting null, without introducing an odd interface to the user.

This is an end-user app, explain to me the difference between "Set to null" and "Delete", there's no difference, so why complicate it and make it less recognizable? what the hell is "null"?

[oSumAtrIX]

Null is just the computer way of saying nothing, a Delete button makes something nothing.

Adding a delete button to the option does not imply setting the option value to null. It implies deleting an option. This semantic is weird. You cant "delete options" you can set the option value to null.

The correct place therefor to add a "Delete" semantic is therefor next to the input label. By your approach we would have to add an "Chose folder" button to the top right of the option card as well, which does not make sense for the same reason as adding a delete button. Instead, the "Delete" functionality, which in reality is a "Set to null" functionality should be added next to the input label, as it is not used to delete the option but to set the option value to null. As there's no space for buttons next to the input label, it is added to the dropdown just like any functionality that is dedicated to setting option values, such as setting the option value to a path, or in this case setting the option value to null

The only issue I see with a "Set to null" submenu item is that an empty string is indistinguishable from a null input.
This could be solved by when pressing on "Set to null", the input value shows a placeholder that could, for example, say "Null value". Once you type something in it, or delete everything in the textbox, it'll be an empty textbox without that placeholder. Here is a mockup:

Actually null:

Empty:

[Ushie]

Adding a delete button to the option does not imply setting the option value to null. It implies deleting an option. This semantic is weird. You cant "delete options" you can set the option value to null.

Please explain to me the different to the user.

[oSumAtrIX]

"Delete option" is a semantical problem. This already disqualifies it as a solution. Null being unclear is a problem, that does not justify using a semantically incorrect problem instead.

Scenario:

You want to change the icon but not the label.
You set label to "null" and the patch wont change it. As you identified, "Null" is unclear to an enduser.

But with delete option you completely break the semantic:

You want to change the icon but not the label.
You "delete the label option?" What does that mean, why is an option "deleteable?" How would the user understand that "Deleting" the label option means not applying a custom label? Here you don't only have a clearness issue, but also the semantic issue

A solution that fixes this can be done on the patches side.
The patch could provide another value such as "Don't change". This fixes the clearness issue, but the current existing semantical problem that "Delete == reset" is not solved. The issue is solved by implementing what I showed in the mockup in my previous problem though.

[oSumAtrIX]

I think the label could say "Value set to null" is better than my previous example. Little more clearer

[Shane9248]

@oSumAtrIX nothing on this so far ?

[Shane9248]

@TheAabedKhan are u aware of this ? as u have been assigned to it