pleroma/pleroma-fe
0
0
Fork 0
Code Issues 343 Pull requests 27 Projects Releases Packages Wiki Activity

Patch to add image descriptions #1722

Closed
andarna wants to merge 2 commits from gitlab-mr-iid-450 into develop
pull from: gitlab-mr-iid-450
merge into: pleroma:develop
Conversation 17 Commits 2 Files changed 8 +38 -11
andarna commented 2019-01-04 15:49:54 +00:00
Member
Copy link

Support for image descriptions in Pleroma Front-End

This patch adds support for image descriptions to Pleroma-FE. The basic functionality is there (adding descriptions) but there is currently no limit on the size of the description.

This addresses issue #136

I have tested this by attaching 1, 2 and 3 image in the Pleroma FE and checking that the alt text shows up on the images in the timeline, as well as in the Mastodon web interface for Pleroma and for Mastodon. I also verified that alt text on existing images shows correctly.

Changes made to the Pleroma Front-End code:

For posting a status with images

  • Added code for text input field in components/post_status_form/post_status_form.vue.

      <div class="img-desc-container">
      <label for="imageDescription">{{$t('post_status.image_description')}}
      </label>
      <input type="text" id="imageDescription"     
          :placeholder="$t('post_status.image_description')"
          v-model="newStatus.imageDescriptions[file.id]"
          class="form-img-desc">
  </div>

  • This required a new entry image_description in i18n/en.json (other languages not done):

    "post_status": {
   ...
  "image_description": "Image description",
   ...
}

  • Added newStatus.imageDescriptions in components/post_status_form/post_status_form.js. I changed addMediaFile() so this map gets initially populated with the filename for each file id:

    addMediaFile (fileInfo) {      
    this.newStatus.files.push(fileInfo)
    this.newStatus.imageDescriptions[fileInfo.id]
      =  fileInfo.url.split('/').pop()
    this.enableSubmit()
},

  • Changed postStatus in src/services/status_poster/status_poster.service.js and src/services/api/api.service.js to include the imageDescriptions map. The apiService adds this to the form to be osted:

      form.append('descriptions', JSON.stringify(imageDescriptions))

For displaying alt text on images

  • Changed the Attachment template in src/components/attachment/attachment.vue to add the image description to the StillImage as title and alt:

      <a v-if="type === 'image' && (!hidden || preloadImage)"
  class="image-attachment"
  :class="{'hidden': hidden && preloadImage}"
  :href="attachment.url"
  target="_blank" :title="attachment.description">
      <StillImage
      :class="{'small': isSmall}"
      v-bind:alt="attachment.description"
      v-bind:title="attachment.description"
      referrerpolicy="no-referrer"
      :mimetype="attachment.mimetype"
      :src="attachment.large_thumb_url || attachment.url"
      />
  </a>

  • Added imageDescription to data() in src/components/attachment/attachment.js but I think it is unused.

  • Changed the StillImage template in src/components/still-image/still-image.vue to add the image description to the StillImage as title and alt:

      <template>
        <div class='still-image' :class='{ animated: animated }' >
          <canvas ref="canvas" v-if="animated"></canvas>
          <img v-bind:alt="description"
          v-bind:title="description"
          ref="src" :src="src"
          :referrerpolicy="referrerpolicy"
          v-on:load="onLoad"
          />
        </div>
  </template>

  • Added description to props: in src/components/still-image/still-image.js

Changes made to the Pleroma Back-End code:

This is a separate patch, pleroma/pleroma#4017

Most of the code was already in place, all I did was change the function attachments_from_ids() in lib/pleroma/web/common_api/utils.ex to take the image descriptions as argument and add them to the name field of the attachments.

def attachments_from_ids(ids, descs_str) do
  {_,descs}=Jason.decode(descs_str)
  Enum.map(ids || [], fn media_id ->
      Map.put(Repo.get(Object, media_id).data,"name",descs[media_id])
  end)
end

The call to this function in post() in CommonAPI (web/common_api/common_api.ex) was changed to take data["descriptions"] as a second argument.

def post(user, %{"status" => status} = data) do
    visibility = get_visibility(data)
    limit = Pleroma.Config.get([:instance, :limit])
    with status <- String.trim(status),
         attachments <- attachments_from_ids(data["media_ids"],data["descriptions"]),
         ...
# Support for image descriptions in Pleroma Front-End This patch adds support for image descriptions to Pleroma-FE. The basic functionality is there (adding descriptions) but there is currently no limit on the size of the description. This addresses issue https://git.pleroma.social/pleroma/pleroma-fe/issues/136 I have tested this by attaching 1, 2 and 3 image in the Pleroma FE and checking that the alt text shows up on the images in the timeline, as well as in the Mastodon web interface for Pleroma and for Mastodon. I also verified that alt text on existing images shows correctly. ## Changes made to the Pleroma Front-End code: ### For posting a status with images - Added code for text input field in `components/post_status_form/post_status_form.vue`. <div class="img-desc-container"> <label for="imageDescription">{{$t('post_status.image_description')}} </label> <input type="text" id="imageDescription" :placeholder="$t('post_status.image_description')" v-model="newStatus.imageDescriptions[file.id]" class="form-img-desc"> </div> - This required a new entry `image_description` in `i18n/en.json` (other languages not done): "post_status": { ... "image_description": "Image description", ... } - Added `newStatus.imageDescriptions` in `components/post_status_form/post_status_form.js`. I changed `addMediaFile()` so this map gets initially populated with the filename for each file id: addMediaFile (fileInfo) { this.newStatus.files.push(fileInfo) this.newStatus.imageDescriptions[fileInfo.id] = fileInfo.url.split('/').pop() this.enableSubmit() }, - Changed `postStatus` in `src/services/status_poster/status_poster.service.js` and `src/services/api/api.service.js` to include the `imageDescriptions` map. The `apiService` adds this to the form to be osted: form.append('descriptions', JSON.stringify(imageDescriptions)) ### For displaying alt text on images - Changed the _Attachment_ template in `src/components/attachment/attachment.vue` to add the image description to the StillImage as _title_ and _alt_: <a v-if="type === 'image' && (!hidden || preloadImage)" class="image-attachment" :class="{'hidden': hidden && preloadImage}" :href="attachment.url" target="_blank" :title="attachment.description"> <StillImage :class="{'small': isSmall}" v-bind:alt="attachment.description" v-bind:title="attachment.description" referrerpolicy="no-referrer" :mimetype="attachment.mimetype" :src="attachment.large_thumb_url || attachment.url" /> </a> - Added `imageDescription` to `data()` in `src/components/attachment/attachment.js` but I think it is unused. - Changed the _StillImage_ template in `src/components/still-image/still-image.vue` to add the image description to the StillImage as _title_ and _alt_: <template> <div class='still-image' :class='{ animated: animated }' > <canvas ref="canvas" v-if="animated"></canvas> <img v-bind:alt="description" v-bind:title="description" ref="src" :src="src" :referrerpolicy="referrerpolicy" v-on:load="onLoad" /> </div> </template> - Added `description` to `props:` in `src/components/still-image/still-image.js` ## Changes made to the Pleroma Back-End code: This is a separate patch, https://git.pleroma.social/pleroma/pleroma/pulls/4017 Most of the code was already in place, all I did was change the function `attachments_from_ids()` in `lib/pleroma/web/common_api/utils.ex` to take the image descriptions as argument and add them to the _name_ field of the attachments. def attachments_from_ids(ids, descs_str) do {_,descs}=Jason.decode(descs_str) Enum.map(ids || [], fn media_id -> Map.put(Repo.get(Object, media_id).data,"name",descs[media_id]) end) end The call to this function in `post()` in _CommonAPI_ (`web/common_api/common_api.ex`) was changed to take `data["descriptions"]` as a second argument. def post(user, %{"status" => status} = data) do visibility = get_visibility(data) limit = Pleroma.Config.get([:instance, :limit]) with status <- String.trim(status), attachments <- attachments_from_ids(data["media_ids"],data["descriptions"]), ...
kaniini commented 2019-01-04 16:38:38 +00:00
Member
Copy link

Screenshot?

Screenshot?
shpuld commented 2019-01-04 21:50:09 +00:00
Owner
Copy link

This probably shouldn't be here?

This probably shouldn't be here?
shpuld commented 2019-01-04 21:52:24 +00:00
Owner
Copy link

Nitpick: would prefer just the vue shorthands :alt or :title instead of using v-bind.

Nitpick: would prefer just the vue shorthands :alt or :title instead of using v-bind.
shpuld commented 2019-01-04 21:55:36 +00:00
Owner
Copy link

any particular reason for switching to typed props here?

any particular reason for switching to typed props here?
shpuld commented 2019-01-04 21:56:28 +00:00
Owner
Copy link

Please remove comments that aren't there to document the code

Please remove comments that aren't there to document the code
shpuld commented 2019-01-04 22:06:30 +00:00
Owner
Copy link

Indenting looks off here.

I haven't yet checked out how it looks (screenshot would be nice), but I have a feeling that label shouldn't be necessary if there's already the placeholder text. Also perhaps better ux would probably be letting user set the descriptions after uploading for each attachment, but I think this way will do at first.

Indenting looks off here. I haven't yet checked out how it looks (screenshot would be nice), but I have a feeling that label shouldn't be necessary if there's already the placeholder text. Also perhaps better ux would probably be letting user set the descriptions after uploading for each attachment, but I think this way will do at first.
shpuld commented 2019-01-04 22:18:01 +00:00
Owner
Copy link

same here with the v-bind

same here with the v-bind
hj commented 2019-01-04 22:19:08 +00:00
Owner
Copy link

What's Wolksvagen has to do with anything?

What's Wolksvagen has to do with anything?
shpuld commented 2019-01-04 22:19:38 +00:00
Owner
Copy link

very wild indents with this block

very wild indents with this block
hj commented 2019-01-04 22:23:55 +00:00
Owner
Copy link

image
image

made some screenshots.

Doesn't look too good but i guess this will do for now. I'd rather have descriptions overlay on image, but, more importantly only show the field after clicking some button like add a pencil next to a cross or something.

![image](/attachments/e1247c0a-281f-4f4f-ba7c-9b1bd05208d2) ![image](/attachments/06155b1c-37ac-4f3f-8556-b39dcfff4135) made some screenshots. Doesn't look too good but i guess this will do for now. I'd rather have descriptions overlay on image, but, more importantly only show the field after clicking some button like add a pencil next to a cross or something.
image.png
90 KiB
image.png
171 KiB
hj commented 2019-01-04 22:25:25 +00:00
Owner
Copy link

At the very least - make so that text field is empty by default, having filename in there is confusing AF. BE should handle filename-as-description as it already does.

At the very least - make so that text field is empty by default, having filename in there is confusing AF. BE should handle filename-as-description as it already does.
hj commented 2019-01-04 22:26:21 +00:00
Owner
Copy link

BE already does filename as default, pre-filling it in FE is confusing.

BE already does filename as default, pre-filling it in FE is confusing.
hj commented 2019-01-04 22:27:34 +00:00
Owner
Copy link

removing pre-filled filename text would indeed make label obsolete

removing pre-filled filename text would indeed make label obsolete
kaniini commented 2019-01-04 22:35:12 +00:00
Member
Copy link

An overlay wouldn't be too hard to do with CSS, but I think we should make the background reasonably solid (say 30% translucency max). There have been many complaints about the Mastodon FE captioning tool not being accessible to colorblind people due to the translucency on their overlay.

An overlay wouldn't be too hard to do with CSS, but I think we should make the background reasonably solid (say 30% translucency max). There have been many complaints about the Mastodon FE captioning tool not being accessible to colorblind people due to the translucency on their overlay.
hj commented 2019-01-04 22:49:49 +00:00
Owner
Copy link

ofc. Theming engine actually gives you hints on how good or bad color is even if it's transparent (it shows how bad contrast is/could get but doesn't factor in the noisiness of background tho)

ofc. Theming engine actually gives you hints on how good or bad color is even if it's transparent (it shows how bad contrast is/could get but doesn't factor in the noisiness of background tho)
andarna commented 2019-01-20 11:28:34 +00:00
Author
Member
Copy link

Just went through this thread. I'm sure any changes all of you have suggested are for the better but for most of them I don't know how to do them. Is there anything I should really change to my patch so that someone else can take it further?
You would do me a great favour if you'd notify me on wim_v12e@cybre.space .

Just went through this thread. I'm sure any changes all of you have suggested are for the better but for most of them I don't know how to do them. Is there anything I should really change to my patch so that someone else can take it further? You would do me a great favour if you'd notify me on wim_v12e@cybre.space .
kaniini commented 2019-02-18 06:27:46 +00:00
Member
Copy link

Superceded by #1864.

Superceded by #1864.

Pull request closed

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

   
Backlog

   
Better behavior

   
Blocked by deps

   
Bug

   
Code Review

   
Discussion

   
Doing

   
Feature Request / Enhancement

   
In the shining bright future maybe

   
It's complicated

   
MS Edge

   
Mememoon (or other niche browsers)

   
Missing API

   
Missing feature

   
Missing l10n/i18n

   
NL1

Instance Admin Dashboard

  
NL2

Improved Mobile Experience

  
NL4

Improved instance customization and QoL options

  
Need to verify on develop

   
RELEASE BLOCKER

   
Reassign or Close

   
Refactor

   
Regression

   
Safari

   
To Do

   
User story

   
Waiting on godot

   
accessibility

https://git.pleroma.social/pleroma/pleroma-fe/-/snippets/7746

  
better documentation

   
chore

   
confirmed

   
easy ticket

   
incident

Denotes a disruption to IT services and the associated issues require immediate attention

  
l10n update

   
mastoapi

   
mobile

   
needs design

   
needs-info

   
needs-review

   
stupid
No labels
BE Request
Backlog
Better behavior
Blocked by deps
Bug
Code Review
Discussion
Doing
Feature Request / Enhancement
In the shining bright future maybe
It's complicated
MS Edge
Mememoon (or other niche browsers)
Missing API
Missing feature
Missing l10n/i18n
NL1
NL2
NL4
Need to verify on develop
RELEASE BLOCKER
Reassign or Close
Refactor
Regression
Safari
To Do
User story
Waiting on godot
accessibility
better documentation
chore
confirmed
easy ticket
incident
l10n update
mastoapi
mobile
needs design
needs-info
needs-review
stupid
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
4 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
pleroma/pleroma-fe!1722
No description provided.