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

Preserve emojis order when adding #2698

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

viktorasl
Copy link
Contributor

@viktorasl viktorasl commented Mar 20, 2019

  • reactions order in app is defined by array in app (not received order from API)
  • reactions picker & UI list share the same data structure

Closes #2403.

When inserting reaction to the middle last cell held UI state applied by pullOut and that's why that had to be extracted to a separate method which has to be called by configure method.

Not sure if private Array extension is correct implementation in this case.

* reactions order in app is defined by array in app (not received order from API)
* reactions picker & UI list share the same data structure
let models: [ReactionViewModel]
private let map: [ReactionContent: ReactionViewModel]
private let flatState: String

init(models: [ReactionViewModel]) {
self.models = models
let reactions = IssueCommentReactionViewModel.allReactions
let sortedModels = models.sorted { (m1, m2) -> Bool in
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit:
instead of m1, m2, why not lhs, rhs?
additionally for m1Idx, m2Idx, why not lhsContent and rhsContent

Copy link
Contributor Author

Choose a reason for hiding this comment

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

lhs & rhs totally makes sense!
Not sure about xContent as it's an index of content in predefined array for order.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@viktorasl what is the status of this? 😊

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh sorry, I thought I've fixed this. On it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So sorry it took so long. Fixed now.

@BasThomas BasThomas added the 💤 awaiting review Pull Request is awaiting code reviews label Mar 20, 2019
@jdisho jdisho added 😴 awaiting changes Changes requested, waiting on author to update and removed 💤 awaiting review Pull Request is awaiting code reviews labels Jun 3, 2019
@jdisho jdisho added 💤 awaiting review Pull Request is awaiting code reviews and removed 😴 awaiting changes Changes requested, waiting on author to update labels Jun 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
💤 awaiting review Pull Request is awaiting code reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adding reaction goes to end of list
3 participants