Skip to content

feat/emote-menu-sorting: Add sorting to emote menu.#516

Closed
berghall wants to merge 33 commits into
SevenTV:masterfrom
berghall:feat/emote-menu-sorting
Closed

feat/emote-menu-sorting: Add sorting to emote menu.#516
berghall wants to merge 33 commits into
SevenTV:masterfrom
berghall:feat/emote-menu-sorting

Conversation

@berghall
Copy link
Copy Markdown
Contributor

The sorting will default to newest added emote.

Closes #398, #511

@AnatoleAM AnatoleAM linked an issue Apr 16, 2023 that may be closed by this pull request
Copy link
Copy Markdown
Contributor

@AnatoleAM AnatoleAM left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This currently breaks one core design point of the emote menu, which is width sort, with wider emotes towards the bottom. This should continue to apply regardless of other sorting checks.

Comment thread src/site/twitch.tv/modules/emote-menu/EmoteMenuContext.ts Outdated
Comment thread src/site/twitch.tv/modules/emote-menu/EmoteMenuModule.vue Outdated
Comment thread src/site/twitch.tv/modules/emote-menu/EmoteMenuSet.vue Outdated
Comment thread src/site/twitch.tv/modules/emote-menu/EmoteMenuModule.vue Outdated
@berghall
Copy link
Copy Markdown
Contributor Author

@AnatoleAM PR updated with request changes. This sorting happens only after sortCase() which handles the widths, so it works as expected and doesn't break the width priority.

@berghall berghall requested a review from AnatoleAM April 17, 2023 14:22
Copy link
Copy Markdown
Contributor

@AnatoleAM AnatoleAM left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Functionality wise this works fine now. In terms of user experience though, this isn't great.

The sorting options should be within the emote menu itself. i.e, next to the search bar.

@berghall
Copy link
Copy Markdown
Contributor Author

@AnatoleAM I guess I should use the font-awesome icons and I need the svg, so could I get them from you?
Was thinking for the order these two https://fontawesome.com/icons/sort-up?f=classic&s=duotone&sc=%231E3050 and https://fontawesome.com/icons/sort-down?f=classic&s=duotone&sc=%231E3050.
For the sort type https://fontawesome.com/icons/bars-sort?f=classic&s=solid

@AnatoleAM
Copy link
Copy Markdown
Contributor

@berghall added the icons on your branch

@berghall
Copy link
Copy Markdown
Contributor Author

@AnatoleAM controls are now next to the search input

f57da1c9bb6d2dfc54d174c756a4611c
17ab7321fdd1768dd852c3e5c856f50c

Live input search:
3159dc97f5c7a18ce158fe18578ea920

@berghall berghall requested a review from AnatoleAM April 20, 2023 18:51
@ayyybubu
Copy link
Copy Markdown
Contributor

Not really a fan of adding more clutter to the menu and shrinking the size of the search bar. All of this could be wrapped inside a single context menu.
dJdEix6

@berghall
Copy link
Copy Markdown
Contributor Author

@ayyybubu
Copy link
Copy Markdown
Contributor

You can just use the bars-sort icon you already have. It doesn't really matter

@berghall
Copy link
Copy Markdown
Contributor Author

Updated
ba0bfe5215f365b1201689b3673b7713

Changed the position when live input search is on, to top of settings button
16dfcf5b9b8d25bf2797b2203f0fe073

@ayyybubu
Copy link
Copy Markdown
Contributor

Very nice so far. I noticed some minor cosmetic issues:

  • extend the clickable area to the whole container (currently only the sort icon is clickable)
  • make the context menu tabs the same width (they are off by a couple of pixels)
  • the default color of the sort icon in the search bar could be var(--seventv-text-color-secondary) and then it would turn into var(--seventv-text-color-normal) when focused (currently the icon really sticks out)

@berghall
Copy link
Copy Markdown
Contributor Author

@ayyybubu your requested changes have been implemented.

@ayyybubu
Copy link
Copy Markdown
Contributor

Thank you, it looks pretty clean now. The last thing that i forgot to mention before:
-clicking on the sort button again should hide the active context menu

@berghall
Copy link
Copy Markdown
Contributor Author

berghall commented Apr 27, 2023

-clicking on the sort button again should hide the active context menu

@ayyybubu this is now fixed.

Comment thread src/site/global/settings/control/FormDropdown.vue Outdated
Comment thread src/site/twitch.tv/modules/emote-menu/EmoteMenuTab.vue Outdated
Comment thread src/site/twitch.tv/modules/emote-menu/EmoteMenu.vue
@berghall
Copy link
Copy Markdown
Contributor Author

@AnatoleAM your requested changes have been implemented.

@berghall berghall requested a review from AnatoleAM May 13, 2023 07:18
@berghall
Copy link
Copy Markdown
Contributor Author

@AnatoleAM merge conflicts resolved

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Jan 24, 2024

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ berghall
❌ AnatoleAM
You have signed the CLA already but the status is still pending? Let us recheck it.

@TroyKomodo
Copy link
Copy Markdown
Contributor

Features against this repo wont be implemented, we are going to rewrite the extension on this repo.

@TroyKomodo TroyKomodo closed this Feb 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[REQUEST] Add sorting emotes by newest added [Request] Option to sort emotes

5 participants