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

Emoji Pack Picker #102

Merged
floatingghost merged 7 commits from Mergan/pleroma-fe:develop into develop 2022-08-03 20:56:57 +00:00
Conversation 34 Commits 7 Files changed 3 +79 -51
Mergan commented 2022-08-02 21:02:32 +00:00
Member
Copy link

Mergan did a thing?

Didn't clean up though

Also emojis load on-demand when selecting an emoji pack, so I didn't think it would be necessary to load in chunks.

Also the matching by keyword might be faster

Aye

Mergan did a thing? Didn't clean up though Also emojis load on-demand when selecting an emoji pack, so I didn't think it would be necessary to load in chunks. Also the matching by keyword might be faster Aye
Mergan added 1 commit 2022-08-02 21:02:33 +00:00
Emoji Picker
All checks were successful
ci/woodpecker/pr/woodpecker Pipeline was successful
Details
8c58cb4033
Mergan commented 2022-08-02 21:13:41 +00:00
Author
Member
Copy link

imageimageimage

![image](/attachments/43ae92b4-8a9a-4fc1-89f7-f34ebb75855d)![image](/attachments/84ced62b-0606-4434-bc6d-1513c136fbb6)![image](/attachments/c116675c-7210-4441-816f-1c80a25489d7)
image.png
39 KiB
image.png
123 KiB
image.png
74 KiB
eris commented 2022-08-02 21:39:07 +00:00
Contributor
Copy link

Hell yeah Mergan let's go

I'll try it out tonight

Hell yeah Mergan let's go I'll try it out tonight
🎉 2
eris added the
Feature
 label 2022-08-02 21:42:46 +00:00
Ghost reviewed 2022-08-02 21:44:58 +00:00
Ghost left a comment
First-time contributor
Copy link

haven't tested it out yet but a few things just from looking around

haven't tested it out yet but a few things just from looking around
src/components/emoji_picker/emoji_picker.js
@ -201,1 +186,3 @@
emojis: filterByKeyword(standardEmojis, trim(this.keyword))
first: {
imageUrl: '',
replacement: '🥴'
Ghost commented 2022-08-02 21:41:37 +00:00
First-time contributor
Copy link

kinda confusing when we're dealing with emoji, i think the default "missing file" sprite or maybe a Ø svg would be appropriate

kinda confusing when we're dealing with emoji, i think the default "missing file" sprite or maybe a Ø svg would be appropriate
Ghost commented 2022-08-02 21:47:52 +00:00
First-time contributor
Copy link

faExclamation, perhaps?

`faExclamation`, perhaps?
Mergan commented 2022-08-02 21:51:13 +00:00
Author
Member
Copy link

The 🥴 emoji is the icon I chose for the Unicode Emoji Pack. I'm not sure what it returns when a missing file happens, but I left that part mostly intact 🤔

That'd be on emoji_picker.vue 16

The 🥴 emoji is the icon I chose for the Unicode Emoji Pack. I'm not sure what it returns when a missing file happens, but I left that part mostly intact 🤔 That'd be on emoji_picker.vue 16
Ghost commented 2022-08-02 21:52:21 +00:00
First-time contributor
Copy link

aaahh, i see. disregard this then

aaahh, i see. disregard this then
Ghost marked this conversation as resolved
src/components/emoji_picker/emoji_picker.js
@ -202,0 +191,4 @@
}
].concat(emojiPacks)
},
sortedEmoji () {
Ghost commented 2022-08-02 21:42:37 +00:00
First-time contributor
Copy link

think this should probably sort alphabetically by pack name instead of by.. it looks like pack size?

think this should probably sort alphabetically by pack name instead of by.. it looks like pack size?
Mergan commented 2022-08-02 21:52:50 +00:00
Author
Member
Copy link

The emojis come alphabetically ordered by default, but they were in a single pack. All I did was sort them into their respective packs. After that they're aplhabetically ordered within their packs (I think). The order of the packs is a first come first serve on the packing lol

The emojis come alphabetically ordered by default, but they were in a single pack. All I did was sort them into their respective packs. After that they're aplhabetically ordered within their packs (I think). The order of the packs is a first come first serve on the packing lol
Ghost commented 2022-08-02 21:56:53 +00:00
First-time contributor
Copy link

yeah i think the order of the pack should be passed through a sort

yeah i think the order of the pack should be passed through a sort
Mergan commented 2022-08-02 21:58:31 +00:00
Author
Member
Copy link

I can add it real quick but hmm how do I hmmm does this undo the pull request or what's the procedure

I can add it real quick but hmm how do I hmmm does this undo the pull request or what's the procedure
Ghost commented 2022-08-02 22:01:03 +00:00
First-time contributor
Copy link

you can send a new commit to the branch and the pr will be updated
or, if you want it to still be one commit, you can git commit --amend and git push --force

you can send a new commit to the branch and the pr will be updated or, if you want it to still be one commit, you can `git commit --amend` and `git push --force`
src/components/emoji_picker/emoji_picker.scss Outdated
@ -73,0 +76,4 @@
overflow-x: scroll;
overflow-y: hidden;
// -ms-overflow-style: none;
Ghost commented 2022-08-02 21:43:26 +00:00
First-time contributor
Copy link

cleanup for later

cleanup for later
Mergan commented 2022-08-02 21:53:27 +00:00
Author
Member
Copy link

Thought it'd look cool without the scroll but I couldn't picture a way to make it intuitive so I left it just in case

Thought it'd look cool without the scroll but I couldn't picture a way to make it intuitive so I left it just in case
Ghost commented 2022-08-02 21:49:28 +00:00
First-time contributor
Copy link

also, suggestion: making the pack titles clickable in order to collapse them. (would require an up/down arrow on the right side for UX)

also, suggestion: making the pack titles clickable in order to collapse them. (would require an up/down arrow on the right side for UX)
Mergan commented 2022-08-02 21:56:10 +00:00
Author
Member
Copy link

also, suggestion: making the pack titles clickable in order to collapse them. (would require an up/down arrow on the right side for UX)

When a pack is selected it only shows that one.
When searching it shows all packs that contain a match (and I got a surprise! Looks like they become highlighted/deactivated if the emoji is not found in the pack)

> also, suggestion: making the pack titles clickable in order to collapse them. (would require an up/down arrow on the right side for UX) When a pack is selected it only shows that one. When searching it shows all packs that contain a match (and I got a surprise! Looks like they become highlighted/deactivated if the emoji is not found in the pack)
Ghost commented 2022-08-02 22:00:04 +00:00
First-time contributor
Copy link

hm
i'm sort of envisioning a very complicated thing where
instead of click pack => only shows pack
it's click pack => widget autoscrolls to pack while still being able to view everything else
which lets people just browse the emoji list more easily

it's probably a bit too overengineered for this though 🥴

hm i'm sort of envisioning a very complicated thing where instead of `click pack => only shows pack` it's `click pack => widget autoscrolls to pack while still being able to view everything else` which lets people just browse the emoji list more easily it's probably a bit too overengineered for this though 🥴
Mergan commented 2022-08-02 22:02:27 +00:00
Author
Member
Copy link

hm
i'm sort of envisioning a very complicated thing where
instead of click pack => only shows pack
it's click pack => widget autoscrolls to pack while still being able to view everything else
which lets people just browse the emoji list more easily

it's probably a bit too overengineered for this though 🥴

Looks like that was the original plan but I thought that was needlesly complex. e.g. loading 60 emojis at a time from all packs, slowly scrolling and matching which one.

I think I removed a bit of that functionality on the highlight function (65) on emoji_picker.js

Thought it would be simpler to just show on-demand. I think this might also fix lag when opening the picker? Because it doesn't load everything

> hm > i'm sort of envisioning a very complicated thing where > instead of `click pack => only shows pack` > it's `click pack => widget autoscrolls to pack while still being able to view everything else` > which lets people just browse the emoji list more easily > > it's probably a bit too overengineered for this though 🥴 Looks like that was the original plan but I thought that was needlesly complex. e.g. loading 60 emojis at a time from all packs, slowly scrolling and matching which one. I think I removed a bit of that functionality on the highlight function (65) on emoji_picker.js Thought it would be simpler to just show on-demand. I think this might also fix lag when opening the picker? Because it doesn't load everything
Ghost commented 2022-08-02 22:07:05 +00:00
First-time contributor
Copy link

yeah that's fair
testing it right now

yeah that's fair testing it right now
Ghost requested changes 2022-08-02 22:08:29 +00:00
Ghost left a comment
First-time contributor
Copy link

nitpick but i think the search makes more sense being right at the top of the widget
it being under the pack picker signifies that it searches within the pack, which it doesn't

i still don't know how i feel about not having an "All" option but meh that's fine

nitpick but i think the search makes more sense being right at the top of the widget it being under the pack picker signifies that it searches within the pack, which it doesn't i still don't know how i feel about not having an "All" option but meh that's fine
Mergan added 1 commit 2022-08-02 22:08:39 +00:00
Sort Custom Emoji Packs by Name
All checks were successful
ci/woodpecker/pr/woodpecker Pipeline was successful
Details
368572fbd0
Mergan requested review from Ghost Team 2022-08-02 22:09:26 +00:00
Mergan commented 2022-08-02 22:10:17 +00:00
Author
Member
Copy link

Ok I pushed the commit did that fix it image

Ok I pushed the commit did that fix it ![image](/attachments/88c3661f-dbc4-46d0-aca7-def0bc7490b9)
image.png
164 KiB
image.png
5.9 KiB
image.png
Ghost commented 2022-08-02 22:12:55 +00:00
First-time contributor
Copy link

Ok I pushed the commit did that fix it image

seems so

> Ok I pushed the commit did that fix it ![image](/attachments/88c3661f-dbc4-46d0-aca7-def0bc7490b9) seems so
Mergan commented 2022-08-02 22:14:36 +00:00
Author
Member
Copy link

nitpick but i think the search makes more sense being right at the top of the widget
it being under the pack picker signifies that it searches within the pack, which it doesn't

i still don't know how i feel about not having an "All" option but meh that's fine

image I don't know, it looks kinda weird image

> nitpick but i think the search makes more sense being right at the top of the widget > it being under the pack picker signifies that it searches within the pack, which it doesn't > > i still don't know how i feel about not having an "All" option but meh that's fine ![image](/attachments/1fdaaa76-a395-45db-b53c-7562a1e269dd) I don't know, it looks kinda weird ![image](/attachments/794ee770-b2a0-4f68-a5aa-801dcab2ec7d)
image.png
8.5 KiB
image.png
123 KiB
Mergan commented 2022-08-02 22:15:48 +00:00
Author
Member
Copy link

nitpick but i think the search makes more sense being right at the top of the widget
it being under the pack picker signifies that it searches within the pack, which it doesn't

i still don't know how i feel about not having an "All" option but meh that's fine
I don't know, it looks kinda weird

image Maybe at the bottom?

> > nitpick but i think the search makes more sense being right at the top of the widget > > it being under the pack picker signifies that it searches within the pack, which it doesn't > > > > i still don't know how i feel about not having an "All" option but meh that's fine > I don't know, it looks kinda weird ![image](/attachments/c33750cf-8754-4f60-bf19-ff095a9a431b) Maybe at the bottom?
image.png
104 KiB
eris commented 2022-08-02 22:36:57 +00:00
Contributor
Copy link

I'm a fan of the current functionality of the search resultsreplacing the emoji list, is that possible? Or at least the size of it taking up the window and not needing horizontal scrolling.

That small little search bar will be hell if I want to search for something with a lot of results.

I'm a fan of the current functionality of the search resultsreplacing the emoji list, is that possible? Or at least the size of it taking up the window and not needing horizontal scrolling. That small little search bar will be hell if I want to search for something with a lot of results.
Mergan commented 2022-08-03 08:35:30 +00:00
Author
Member
Copy link

I'm a fan of the current functionality of the search resultsreplacing the emoji list, is that possible? Or at least the size of it taking up the window and not needing horizontal scrolling.

That small little search bar will be hell if I want to search for something with a lot of results.

Could ye rephrase that? I didn't quite catch it 😅

> I'm a fan of the current functionality of the search resultsreplacing the emoji list, is that possible? Or at least the size of it taking up the window and not needing horizontal scrolling. > > That small little search bar will be hell if I want to search for something with a lot of results. Could ye rephrase that? I didn't quite catch it 😅
Mergan commented 2022-08-03 09:54:03 +00:00
Author
Member
Copy link

AkkomaGang/pleroma-fe#69 Oops reference

AkkomaGang/pleroma-fe#69 Oops reference
floatingghost reviewed 2022-08-03 13:52:29 +00:00
floatingghost left a comment
Owner
Copy link

ok my thoughts - it works pretty much as i'd personally expect

the unicode emoji currently clips the scrollbar, so probably needs some internal padding

also comment attached

otherwise is decent|

ok my thoughts - it works pretty much as i'd personally expect the unicode emoji currently clips the scrollbar, so probably needs some internal padding also comment attached otherwise is decent|
src/components/emoji_picker/emoji_picker.js Outdated
@ -190,0 +174,4 @@
customEmojis.forEach((pack, id) => {
emojiPacks.push({
id: id.substring(5),
text: startCase(id.substring(5)),
floatingghost commented 2022-08-03 13:51:49 +00:00
Owner
Copy link

this here assumes pack: name format, which isn't true for some older emojis - check that id starts with pack: then chop it, or keep as is

for example: https://ihatebeinga.live/api/pleroma/emoji.json

see "aiwave", which is the old-style local pack and doesn't have a pack prefix

this here assumes `pack:` name format, which isn't true for some older emojis - check that id starts with `pack:` then chop it, or keep as is for example: https://ihatebeinga.live/api/pleroma/emoji.json see "aiwave", which is the old-style local pack and doesn't have a `pack` prefix
floatingghost closed this pull request 2022-08-03 13:58:28 +00:00
floatingghost reopened this pull request 2022-08-03 13:58:35 +00:00
Mergan commented 2022-08-03 14:00:26 +00:00
Author
Member
Copy link

No worries comrade.
What follows now?

No worries comrade. What follows now?
floatingghost commented 2022-08-03 14:01:06 +00:00
Owner
Copy link

the review above probably needs a look over, since some pack formats break

otherwise i think this is gud

the review above probably needs a look over, since some pack formats break otherwise i think this is gud
Mergan added 1 commit 2022-08-03 14:08:08 +00:00
Replace pack with regex instead of assuming "pack:"
All checks were successful
ci/woodpecker/pr/woodpecker Pipeline was successful
Details
f85841aa80
Mergan commented 2022-08-03 14:08:58 +00:00
Author
Member
Copy link

image Uhh

![image](/attachments/52d9d38c-b196-4616-b74e-f3fbc10ffd12) Uhh
image.png
5.7 KiB
floatingghost commented 2022-08-03 14:09:28 +00:00
Owner
Copy link

oh that's cool never mind about that, there's no conflict

oh that's cool never mind about that, there's no conflict
floatingghost commented 2022-08-03 14:09:43 +00:00
Owner
Copy link

just the unicode padding to remove the scrollbar clip and i'll merge~

just the unicode padding to remove the scrollbar clip and i'll merge~
Mergan commented 2022-08-03 14:10:38 +00:00
Author
Member
Copy link

just the unicode padding to remove the scrollbar clip and i'll merge~

It's the pack representative, right? do ye have an image of how it shows currently? (for reference)

> just the unicode padding to remove the scrollbar clip and i'll merge~ It's the pack representative, right? do ye have an image of how it shows currently? (for reference)
floatingghost commented 2022-08-03 14:54:07 +00:00
Owner
Copy link

current:
image

expected:
image

setting vertical-align: top on emoji-tabs-item appears to fix this

current: ![image](/attachments/a22c0280-75be-476e-a108-48b5016d05e6) expected: ![image](/attachments/f1703d8b-c937-42e0-8b2e-f6824db2a74a) setting `vertical-align: top` on `emoji-tabs-item` appears to fix this
image.png
69 KiB
image.png
41 KiB
Mergan commented 2022-08-03 14:56:04 +00:00
Author
Member
Copy link

image Remaking the css a bit to make it cleaner and more consistent. I'm just getting this weird bug with the scrollbar, any ideas?

![image](/attachments/7d02cb5d-a087-4b15-889a-0abd9435dc59) Remaking the css a bit to make it cleaner and more consistent. I'm just getting this weird bug with the scrollbar, any ideas?
image.png
16 KiB
Mergan added 1 commit 2022-08-03 15:07:26 +00:00
CSS Cleanup
All checks were successful
ci/woodpecker/pr/woodpecker Pipeline was successful
Details
f351e2e8dc
floatingghost commented 2022-08-03 15:32:16 +00:00
Owner
Copy link

image

BIG

idk what changed but it seems to now flow massively over and doesn't scrollbar at all

which is especially weird for notifications:
image

![image](/attachments/125ed580-4a99-46f2-9e10-e5bead19092a) BIG idk what changed but it seems to now flow massively over and doesn't scrollbar at all which is especially weird for notifications: ![image](/attachments/60c3f9bf-a161-4750-91bb-98a8f649f027)
image.png
116 KiB
image.png
170 KiB
Mergan commented 2022-08-03 15:33:18 +00:00
Author
Member
Copy link

Oops let me fix that

Oops let me fix that
Mergan added 1 commit 2022-08-03 15:43:09 +00:00
Scrollbar fix
All checks were successful
ci/woodpecker/pr/woodpecker Pipeline was successful
Details
bc36608a6a
Mergan commented 2022-08-03 15:48:38 +00:00
Author
Member
Copy link

Something something display: block → flex
Looking into it

Something something display: block → flex Looking into it
Mergan commented 2022-08-03 17:00:01 +00:00
Author
Member
Copy link

The easiest solution would be to put a width but I have no idea what it should be

The easiest solution would be to put a width but I have no idea what it should be
floatingghost commented 2022-08-03 17:01:15 +00:00
Owner
Copy link

the % is based on the outer container - so max-width: 100% should work i think

the `%` is based on the outer container - so `max-width: 100%` _should_ work i think
Mergan commented 2022-08-03 17:02:09 +00:00
Author
Member
Copy link

It doesn't :s

It doesn't :s
floatingghost commented 2022-08-03 17:12:13 +00:00
Owner
Copy link

ok found why -

in react_button.vue, in the .popover styles, set max-width: 100%

ok found why - in `react_button.vue`, in the `.popover` styles, set `max-width: 100%`
Mergan commented 2022-08-03 17:16:24 +00:00
Author
Member
Copy link

Experimenting...

Experimenting...
Mergan added 1 commit 2022-08-03 17:29:07 +00:00
Added sins
All checks were successful
ci/woodpecker/pr/woodpecker Pipeline was successful
Details
a28eef7a0a
Mergan commented 2022-08-03 17:29:27 +00:00
Author
Member
Copy link

Noone must know of the sins I commited today image

Noone must know of the sins I commited today ![image](/attachments/12e34bc6-24ee-43c6-b88c-4624f24094e8)
image.png
3.1 KiB
Mergan commented 2022-08-03 17:34:05 +00:00
Author
Member
Copy link

I wonder if that breaks stickers 🤔

I wonder if that breaks stickers 🤔
Mergan added 1 commit 2022-08-03 17:46:34 +00:00
Fixed Scrollbar (again)
All checks were successful
ci/woodpecker/pr/woodpecker Pipeline was successful
Details
302d07af35
Mergan commented 2022-08-03 17:46:43 +00:00
Author
Member
Copy link

That should do it

That should do it
floatingghost commented 2022-08-03 20:56:50 +00:00
Owner
Copy link

yep, that works nicely!

thanks a loooot~

yep, that works nicely! thanks a loooot~
❤️ 1
floatingghost merged commit aff6caa4c0 into develop 2022-08-03 20:56:57 +00:00
floatingghost referenced this pull request from a commit 2022-08-03 20:56:57 +00:00
Emoji Pack Picker (#102)
eris referenced this pull request from a commit 2022-08-05 23:53:14 +00:00
Emoji Pack Picker (#102)
Beefox referenced this pull request from a commit 2022-09-22 00:57:34 +00:00
Emoji Pack Picker (#102)
Sign in to join this conversation.
Reviewers
No reviewers
Ghost
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
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: AkkomaGang/akkoma-fe#102
No description provided.
Delete branch "Mergan/pleroma-fe:develop"

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?