Skip to content
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

Added switch to mark new posts as NSFW / toggle tag on existing posts #833

Merged
merged 5 commits into from
Jun 23, 2023

Conversation

sockenklaus
Copy link
Contributor

@sockenklaus sockenklaus commented Jun 22, 2023

Closes #679

Added a switch to CreatePost.kt and PostEdit.kt and added logic to the corresponding activities.

Screenshot_20230623-002021
Screenshot_20230623-002029

… NSFW and change the tag of an existing post. Added logic in the corresponding Activities.

Closes LemmyNet#679
@sockenklaus sockenklaus marked this pull request as ready for review June 22, 2023 22:32
Copy link
Contributor

@twizmwazin twizmwazin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally think a checkbox is more appropriate here. I checked the material guidelines and they seem to prefer a checkbox in cases where it is part of a form. Switches are for things that take immediate effect in the UI (maybe we should refactor settings...). Consider right aligning it too and see if that looks good. Additionally, I think "Mark post as NSFW" might be better than just "NSFW".

@sockenklaus
Copy link
Contributor Author

I personally think a checkbox is more appropriate here. I checked the material guidelines and they seem to prefer a checkbox in cases where it is part of a form. Switches are for things that take immediate effect in the UI (maybe we should refactor settings...).

Would like to show me your source? According to Material.io "Switches are the preferred way to adjust settings. They're used to control binary options – think On/Off or True/False." furthermore "Use switches to turn states on or off, specifically on mobile instead of a checkbox".

According to checkboxes, Material.io says: "Use checkboxes to [...] Select one or more options from a list [...] Turn an item on or off in a desktop environment. Checkboxes should be used instead of switches if multiple options can be selected from a list".

I read this in favour of Switches in this situation.

Consider right aligning it too and see if that looks good.

Agreed! I will try both: Arrangement.SpaceBetween as well as switching control and label but aligning both left. Both should be better than the current arrangement.

Additionally, I think "Mark post as NSFW" might be better than just "NSFW".

Agreed!

@twizmwazin
Copy link
Contributor

Would like to show me your source? According to Material.io "Switches are the preferred way to adjust settings. They're used to control binary options – think On/Off or True/False." furthermore "Use switches to turn states on or off, specifically on mobile instead of a checkbox".

I think the difference in interpretation is that a form and a setting are two different things. They both have binary fields, but the difference is that toggling an option in a form isn't toggling the state. No state is actually created until the form is submitted. On https://m3.material.io/components/switch/guidelines it states:

Use switches to:

  • Toggle a single item on or off
  • Immediately activate or deactivate something

This language leads me to feel it is geared towards settings rather than form fields. On the same page:

Alternate selection controls
Checkboxes and radio buttons are alternative selection controls that can be used to enter decisions or declare preferences such as settings or dialogs.

This language pushes me towards preferring a checkbox in cases where the input my be a "decision".

I see the validity in both of our interpretations and I appreciate you explaining how you came to your conclusion. If the wording and layout changes are implemented I'm fine with a toggle switch, though if anyone else has an experience or opinion to offer on the subject I think it would be welcome.

@sockenklaus
Copy link
Contributor Author

sockenklaus commented Jun 23, 2023

I think the difference in interpretation is that a form and a setting are two different things. They both have binary fields, but the difference is that toggling an option in a form isn't toggling the state. No state is actually created until the form is submitted.

Oh I see that now. I did a little research on UX regarding switches and checkboxes and all articles I read clearly support your stance that switches are to be used when you want to communicate an instantaneous state change and checkboxes are preference when the decision takes effect after a further action (button press). I also get what you meant by "state change" vs. "the state is created when the form is submitted" in this context.

I see the validity in both of our interpretations and I appreciate you explaining how you came to your conclusion.
Thank you, that's very kind of you and I appreciate you taking the time to discuss this with me. I hope I didn't come off as pretentious asking for further explanation.

I will change the PR tonight and address the layout changes.

…ved composable out of CreatePost.kt and PostEdit.kt to reduce unnecessary redundancy.
@sockenklaus
Copy link
Contributor Author

sockenklaus commented Jun 23, 2023

Here the current changes. I also moved the new option as its own composable out of CreatePost and PostEdit. I feel like this could be done with some more elements to reduce unnecessary redundancy.

Maybe I find the time later the day and create its own PR.

Screenshot_20230623-155538__01

@twizmwazin twizmwazin self-requested a review June 23, 2023 17:38
@twizmwazin twizmwazin enabled auto-merge (squash) June 23, 2023 17:46
@twizmwazin twizmwazin merged commit 5eae33d into LemmyNet:main Jun 23, 2023
@sockenklaus sockenklaus deleted the 679_mark_new_post_as_nsfw branch June 23, 2023 21:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add option to mark a new post NSFW
2 participants