Skip to content
Snippets Groups Projects

Emoji selector update

Merged HJ requested to merge emoji-selector-update into develop

successor to !736 (closed)

closes #101 (closed)

Edited by HJ

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • HJ
  • HJ added 1 commit

    added 1 commit

    • 2237da01 - Apply suggestion to src/components/emoji_input/emoji_input.js

    Compare with previous version

  • HJ added 1 commit

    added 1 commit

    Compare with previous version

  • HJ added 1 commit

    added 1 commit

    • 9146bee7 - better hitbox for status emoji

    Compare with previous version

  • Author Maintainer

    Current observations/bugs

    • Image context menu doesn't work on status emojis due to pointer-events: none, gonna fix it with another invisible image in
    • Emoji zoom in picker is somewhat weird on mobile, gonna fix it with better touch handling?
  • Shpuld Shpludson
  • Shpuld Shpludson
    • Resolved by HJ

      emoji picker itself looks and works nice, tried on your instance both in firefox and chromium desktop and firefox mobile. I would however axe the hover to zoom, it's distracting (especially in the picker) on desktop when I tried it and doesn't quite work properly on mobile at all. Interesting idea but doesn't work well in practice imo.

  • Shpuld Shpludson
  • HJ added 1 commit

    added 1 commit

    • 96512939 - Apply suggestion to src/components/emoji_picker/emoji_picker.js

    Compare with previous version

  • HJ added 1 commit

    added 1 commit

    • 4f0195b0 - Apply suggestion to src/components/emoji_picker/emoji_picker.vue

    Compare with previous version

  • 1
    2 const filterByKeyword = (list, keyword = '') => {
    3 return list.filter(x => x.displayText.includes(keyword))
    4 }
    5
    6 const EmojiPicker = {
    7 props: {
    8 stickerPicker: {
    9 required: false,
    10 type: Boolean,
    11 default: false
    12 }
    13 },
    14 data () {
    15 return {
    16 labelKey: String(Math.random() * 100000),
    • wait what

    • Author Maintainer

      I'm gonna make a separate "Checkbox" component some time later. This is a workaround to make unique IDs for now

      Basically, the situation is:

      We have the checkbox that looks like this

      <input type="checkbox" id="spamMode"/>
      <label for="spamMode">Spam mode!</label>

      but then we have multiple new post forms like

      <!-- main form -->
      <input type="checkbox" id="spamMode"/>
      <label for="spamMode">Spam mode!</label>
      
      <!-- reply form -->
      <input type="checkbox" id="spamMode"/>
      <label for="spamMode">Spam mode!</label>

      ruh-oh! we have two inputs with same id!! this breaks checkboxes since you need to click on label to toggle them. And how it works in browser that click goes to first, or last, or whatever id it finds, the behavior is not defined since ids are supposed to be unique:

      <!-- main form -->
      <input type="checkbox" id="spamMode"/> <!-- event arrives here -->
      <label for="spamMode">Spam mode!</label>
      
      <!-- reply form -->
      <input type="checkbox" id="spamMode"/>
      <label for="spamMode">Spam mode!</label> <!-- user clicks here -->

      Ideally we should have a Checkbox component that will generate random ids for us since they are not as important nowadays, i'll try to do that soon after. For now i just add a random number to the id and for attributes like so:

      <!-- main form -->
      <input type="checkbox" id="spamMode-233312"/>
      <label for="spamMode-233312">Spam mode!</label>
      
      <!-- reply form -->
      <input type="checkbox" id="spamMode-312415"/>
      <label for="spamMode-312415">Spam mode!</label>
    • maybe we should have a super simple global serial id counter instead of random? basically just a service that you ask for an id and it just increments it by one every time someone uses it. I know it's very unlikely but there's still the chance of getting the same id twice with random for some very fun bugs

    • Author Maintainer

      i think so, yeah, or use UUID instead, but I plan to do that in a Checkbox component, in a separate MR since this one is bloated enough already.

      Random should work enough, I think we already have same problem with "mark as sensitive" checkbox too anyway.

    • Please register or sign in to reply
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Please register or sign in to reply
    Loading