Skip to content
This repository has been archived by the owner on Sep 20, 2023. It is now read-only.

Adding reaction goes to end of list #2403

Open
Sherlouk opened this issue Nov 4, 2018 · 9 comments · May be fixed by #2698
Open

Adding reaction goes to end of list #2403

Sherlouk opened this issue Nov 4, 2018 · 9 comments · May be fixed by #2698
Labels
🎨 design Changes to the design of the app ✨ good first issue An issue suitable for someone looking to get familiar with the codebase

Comments

@Sherlouk
Copy link
Member

Sherlouk commented Nov 4, 2018

When a reaction is added (tested using double tap to add, haven't validated for normal tap to add) it gets added to the end of list of reactions.

If I already have a reaction (e.g. 🎉) and then add one (e.g. 👍) it will be ordered 🎉 👍 in GitHawk UI. If I leave the issue and then re-enter it, it gets ordered based on a hardcoded array so that it renders 👍 🎉 (this is the same as GitHub's UI).

It's a very minor bug but when we insert a new reaction, we should place it respecting the order of reactions as determined in this array:

private let reactions: [ReactionContent] = [
.thumbsUp,
.hooray,
.thumbsDown,
.heart,
.laugh,
.confused
]

@Huddie
Copy link
Collaborator

Huddie commented Nov 4, 2018

Agree^^^^^^^

Sent with GitHawk

@rnystrom rnystrom added the 🎨 design Changes to the design of the app label Nov 4, 2018
@j-f1
Copy link

j-f1 commented Nov 5, 2018

Happens with picking from the emoji menu too.

@BasThomas BasThomas added the ✨ good first issue An issue suitable for someone looking to get familiar with the codebase label Nov 5, 2018
@Architgoel4
Copy link

Hi, I'd like to pick this issue. But I'm not sure I fully understand this problem. Do we have to arrange the array based on the reactions array (ReactionMenuViewController) or based on what the user entered?
Also, it's my first time contributing to GitHub projects. I have intermediate swift knowledge.

@Huddie
Copy link
Collaborator

Huddie commented Jan 29, 2019

@Architgoel4 When a reaction is added, it should respect the order of the array mentioned above. Not the order it was added but the order determined by Github (and shown in that array)

@Architgoel4
Copy link

Okay, thanks @Huddie! I'll take a look at this then.

@Huddie
Copy link
Collaborator

Huddie commented Mar 13, 2019

@Architgoel4 still on this?

Sent with GitHawk

@Architgoel4
Copy link

I am sorry, I got held up so couldn't even look at it. I'll probably won't be able to look into this for some time.

@BasThomas
Copy link
Collaborator

No worries @Architgoel4, thanks for the update!

I was wondering if this is done together with #2616 already? @viktorasl?

@viktorasl
Copy link
Contributor

Nope, it is not.

@viktorasl viktorasl linked a pull request Mar 20, 2019 that will close this issue
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🎨 design Changes to the design of the app ✨ good first issue An issue suitable for someone looking to get familiar with the codebase
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants