AkkomaGang/akkoma-fe
5
13
Fork 64
Code Issues 81 Pull requests 19 Projects Releases 1 Wiki Activity

Replace "Always show content warning" option with Mastodon-styled content warning button #293

Closed
s0ulf3re wants to merge 12 commits from s0ulf3re/akkoma-fe:s0ulf3re/contentWarningRework into develop
pull from: s0ulf3re/akkoma-fe:s0ulf3re/contentWarningRework
merge into: AkkomaGang:develop
Conversation 2 Commits 12 Files changed 4 +42 -10
s0ulf3re commented 2023-03-22 23:52:18 +00:00
First-time contributor
Copy link

So I decided to rewrite a little bit of the Akkoma Frontend to include an actual content warning button much like on Mastodon. I thought it looked a little odd to have the content warning out all the time. I'm sorry if this isn't something that's wanted. I just decided to work on it to learn the code a little bit and figured that since it seems to be working so far (tested with the main panel), I would at least upload it here.

Basically, this pull request removes the "always show content warning" option from settings. And in it's place, a content warning button is added directly under the post field. When it's clicked, the field then appears and can have a warning typed in. When it isn't shown, nothing is sent.

So I decided to rewrite a little bit of the Akkoma Frontend to include an actual content warning button much like on Mastodon. I thought it looked a little odd to have the content warning out all the time. I'm sorry if this isn't something that's wanted. I just decided to work on it to learn the code a little bit and figured that since it seems to be working so far (tested with the main panel), I would at least upload it here. Basically, this pull request removes the "always show content warning" option from settings. And in it's place, a content warning button is added directly under the post field. When it's clicked, the field then appears and can have a warning typed in. When it isn't shown, nothing is sent.
s0ulf3re added 11 commits 2023-03-22 23:52:19 +00:00
Merge pull request 'port MFM link into stable docs' (#38) from develop into stable
All checks were successful
ci/woodpecker/push/woodpecker Pipeline was successful
Details
d03872d598 
Reviewed-on: AkkomaGang/pleroma-fe#38
Merge pull request 'stable release' (#130) from develop into stable
All checks were successful
ci/woodpecker/push/woodpecker Pipeline was successful
Details
5972d89117 
Reviewed-on: AkkomaGang/pleroma-fe#130
Merge pull request '2022.09 stable' (#160) from develop into stable
All checks were successful
ci/woodpecker/push/woodpecker Pipeline was successful
Details
d7499a1f91 
Reviewed-on: AkkomaGang/pleroma-fe#160
Merge pull request '2022.10 stable' (#177) from develop into stable
All checks were successful
ci/woodpecker/push/woodpecker Pipeline was successful
Details
c8c8d40827 
Reviewed-on: AkkomaGang/pleroma-fe#177
Merge pull request '2022.11 stable release' (#202) from develop into stable
All checks were successful
ci/woodpecker/push/woodpecker Pipeline was successful
Details
975f04bf5a 
Reviewed-on: AkkomaGang/pleroma-fe#202
Merge pull request 'hotfix: translation' (#207) from develop into stable
All checks were successful
ci/woodpecker/push/woodpecker Pipeline was successful
Details
80a519d7e4 
Reviewed-on: AkkomaGang/pleroma-fe#207
Merge pull request 'hotfix: mfm mysteries' (#215) from develop into stable
All checks were successful
ci/woodpecker/push/woodpecker Pipeline was successful
Details
2c9b73646c 
Reviewed-on: AkkomaGang/pleroma-fe#215
Merge branch 'develop' into stable
All checks were successful
ci/woodpecker/push/woodpecker Pipeline was successful
Details
9c9b4cc07c
Merge branch 'develop' into stable
All checks were successful
ci/woodpecker/push/woodpecker Pipeline was successful
Details
8569b5946e
Merge branch 'develop' into stable
All checks were successful
ci/woodpecker/push/woodpecker Pipeline was successful
Details
ci/woodpecker/tag/woodpecker Pipeline was successful
Details
85abc62213
Removed "Always show Content Warning" option
All checks were successful
ci/woodpecker/pr/woodpecker Pipeline was successful
Details
cd5a09935e
s0ulf3re added 1 commit 2023-03-26 23:54:19 +00:00
Merge branch 'develop' into s0ulf3re/contentWarningRework
All checks were successful
ci/woodpecker/pr/woodpecker Pipeline was successful
Details
2713ca0c3e
floatingghost reviewed 2023-03-30 10:11:58 +00:00
floatingghost left a comment
Owner
Copy link

i actually like the general style of this

i've got some feedback on the code itself, but there's also the question of how to make it consistent within the application - the hide image icon that appears over content-warning images is currently a cross, you might want to switch this to an eye to unify it with the icon chosen for the CW input itself

i actually like the general style of this i've got some feedback on the code itself, but there's also the question of how to make it consistent within the application - the `hide image` icon that appears over content-warning images is currently a `cross`, you might want to switch this to an eye to unify it with the icon chosen for the CW input itself
src/components/post_status_form/post_status_form.js
@ -100,6 +100,7 @@ const PostStatusForm = {
'isRedraft'
],
emits: [
'contentWarningFieldVisible',
floatingghost commented 2023-03-30 10:09:38 +00:00
Owner
Copy link

i don't think you're actually emitting this event anywhere, so you can probably remove this

i don't think you're actually emitting this event anywhere, so you can probably remove this
src/components/post_status_form/post_status_form.js
@ -679,0 +689,4 @@
showContentWarningField() {
if (this.showingContentWarning === true) {
console.log(this.showingContentWarning)
this.$refs['contentWarningField'].focus()
floatingghost commented 2023-03-30 10:06:55 +00:00
Owner
Copy link

due to the field being hidden under a v-if, we'll need to wait for the next rendering tick for focus to do anything

also, because you check the show condition before switching it, it'll try and focus when it's not visible

this.showingContentWarning = !this.showingContentWarning
if (this.showingContentWarning) {
  this.$nextTick(() => this.$refs['contentWarningField'].focus())
}

should solve this i think

due to the field being hidden under a v-if, we'll need to wait for the next rendering tick for `focus` to do anything also, because you check the show condition _before_ switching it, it'll try and focus when it's not visible ```javascript this.showingContentWarning = !this.showingContentWarning if (this.showingContentWarning) { this.$nextTick(() => this.$refs['contentWarningField'].focus()) } ``` should solve this i think
src/components/post_status_form/post_status_form.vue
@ -263,0 +272,4 @@
<!-- Content Warning Button -->
<button
class="emoji-icon button-unstyled"
:title="$t('Add content warning')"
floatingghost commented 2023-03-30 10:00:02 +00:00
Owner
Copy link

translated strings are done via keys into the i18n json - see src/i18n/en.json

so you'd probably want this to be $('post_status.add_content_warning_button') or similar

translated strings are done via keys into the i18n json - see src/i18n/en.json so you'd probably want this to be `$('post_status.add_content_warning_button')` or similar
xarvos commented 2023-04-09 02:03:46 +00:00
Contributor
Copy link

this pull request removes the "always show content warning" option from settings

Can we keep both? We can have "always show content warning" on -> the field always shows, no button and "always show content warning" off -> the field is hidden until you click the button.

One of the things I like about Pleroma/Akkoma FE is its customizability; I don't find it necessary to remove this option.

> this pull request removes the "always show content warning" option from settings Can we keep both? We can have "always show content warning" on -> the field always shows, no button and "always show content warning" off -> the field is hidden until you click the button. One of the things I like about Pleroma/Akkoma FE is its customizability; I don't find it necessary to remove this option.
Mergan added the
Feature
 label 2023-04-12 00:32:57 +00:00
floatingghost commented 2023-12-20 18:50:02 +00:00
Owner
Copy link

superceded by #362

superceded by #362
floatingghost closed this pull request 2023-12-20 18:50:02 +00:00
All checks were successful
ci/woodpecker/pr/woodpecker Pipeline was successful
Details

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
a11y

Issues and PRs about accessibility

  
Bug

Applies to issues describing bugs

  
Bug fix

Pull requests that squash bugs

  
Critical Priority

   
Documentation

  
Feature

Pull requests that implement new features

  
Feature request

Issues that request new features

  
Held for next release cycle

PR reviewed during release candidate phase, approved and held as PR

  
High Priority

   
Low Priority

   
Medium Priority

   
Minor change

For minor changes, ie CSS alignment changes

  
Translation/Locale

For changes related to translations

  
WIP

Work in progress

No labels
a11y
Bug
Bug fix
Critical Priority
Documentation
Feature
Feature request
Held for next release cycle
High Priority
Low Priority
Medium Priority
Minor change
Translation/Locale
WIP
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: AkkomaGang/akkoma-fe#293
No description provided.
Delete branch "s0ulf3re/akkoma-fe:s0ulf3re/contentWarningRework"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?