Improve UX of subject / Content Warning field #362

hazelnoot · 2023-12-16T20:26:31Z

hazelnoot commented

2023-12-16 20:26:31 +00:00

This PR implements two changes to improve user experience around the Content Warning field.

First, the duplicate emoji button is removed in favor of the main one. State variables are added to record the most recently focused text box. I attempted to preserve the existing code as much as possible, including re-focusing the appropriate input before showing the picker. I'm not sure if that's actually required, but it's consistent with the existing behavior.

Secondly, a "toggle CW" button is added to toggle visibility of the CW field. This respects the "always show subject" and "disable subject" properties, and also ensures that the CW is cleared after hiding the field. The CW field is automatically shown whenever it has a default value, such as replying to a CW-ed post. The toggle CW button is added to the end of the posting controls, right after the polls button. I tested various positions and found that this one looked the best visually.

I think this is partially a duplicate of #293, but I didn't see that PR until after I completed this work. Oh well. 🤷‍♀️

Before:

After:

This PR implements two changes to improve user experience around the Content Warning field. First, the duplicate emoji button is removed in favor of the main one. State variables are added to record the most recently focused text box. I attempted to preserve the existing code as much as possible, including re-focusing the appropriate input before showing the picker. I'm not sure if that's actually required, but it's consistent with the existing behavior. Secondly, a "toggle CW" button is added to toggle visibility of the CW field. This respects the "always show subject" and "disable subject" properties, and also ensures that the CW is cleared after hiding the field. The CW field is automatically shown whenever it has a default value, such as replying to a CW-ed post. The toggle CW button is added to the end of the posting controls, right after the polls button. I tested various positions and found that this one looked the best visually. I think this is partially a duplicate of #293, but I didn't see that PR until *after* I completed this work. Oh well. 🤷‍♀️ Before: ![fABceu0CIIQRBg](/attachments/63018e9e-d1fe-49ee-8091-6f1e8e6f84a8) After: ![Dur0jtqujnZcxA](/attachments/7a147fb9-c7ff-4d59-ba21-90762637f120) ![9Y-wHP61m32lbw](/attachments/af6d53ed-54d4-4774-a075-ae80715c3b83)

9Y-wHP61m32lbw.png

41 KiB

Dur0jtqujnZcxA.png

39 KiB

fABceu0CIIQRBg.png

41 KiB

❤️ 1

hazelnoot added 2 commits 2023-12-16 20:26:32 +00:00

use main emoji button for spoiler / CW field 3d65eccf04

add button to toggle the spoiler / CW field

ci/woodpecker/pr/woodpecker Pipeline was successful

Details

34e2800f59

hazelnoot commented

2023-12-16 20:27:48 +00:00

By the way, I'm sorry if I missed any important steps! I couldn't find any contributing docs or guidelines.

floatingghost reviewed 2023-12-20 17:07:45 +00:00

floatingghost left a comment

i like the idea a lot, just one small bug to address and i'll be happy to merge this

thanks!

i like the idea a lot, just one small bug to address and i'll be happy to merge this thanks!

src/components/post_status_form/post_status_form.js Outdated

					
				@ -201,1 +201,4 @@

				    // When first loading the form, hide the subject (CW) field if it's disabled or doesn't have a starting value.

				    // "disableSubject" seems to take priority over "alwaysShowSubject"

				    const showSubject = !this.disableSubject && (statusParams.spoilerText || this.alwaysShowSubject)

floatingghost commented

2023-12-20 17:07:24 +00:00

this doesn't quite work due to the technicalities of how vue generates component data

this is in data(), which runs to generate a bunch of initial data for a component

you're relying on computed values here, which have not actually run at this point, meaning that alwaysShowSubject will always be undefined at this point in the lifecycle!

if you were to use this.$store.getters.mergedConfig.alwaysShowSubjectInput instead, this would resolve correctly

this doesn't quite work due to the technicalities of how vue generates component data this is in `data()`, which runs to generate a bunch of initial data for a component you're relying on `computed` values here, which have not actually run at this point, meaning that `alwaysShowSubject` will always be undefined at this point in the lifecycle! if you were to use `this.$store.getters.mergedConfig.alwaysShowSubjectInput` instead, this would resolve correctly

hazelnoot commented

2023-12-20 17:30:38 +00:00

good catch, thank you! I'll push a fix for that.

hazelnoot marked this conversation as resolved

hazelnoot added 1 commit 2023-12-20 17:40:40 +00:00

fix "always show content warning" setting

ci/woodpecker/pr/woodpecker Pipeline was successful

Details

ea9ad4d600

hazelnoot commented

2023-12-20 17:41:23 +00:00

@floatingghost fixed the bug you found. Thanks for reviewing!