Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

First contact load fix, direct access to contact, contact not in group, group load and order - fix #312 #323

Merged
merged 1 commit into from
Apr 5, 2016
Merged

First contact load fix, direct access to contact, contact not in group, group load and order - fix #312 #323

merged 1 commit into from
Apr 5, 2016

Conversation

skjnldsv
Copy link
Contributor

Fix multiple issues:

  • When loading the page directly on a specific group, the first contact loaded was taken from the main contact list which often results by displaying a contact not in the requested group
  • When reclicking the already selected group from the list after a fresh page load resulted by the contact details disappearing
  • The contacts starting with a special character was being ignored when choosing the first contact to load
  • When a uid is specified, the script will now be looking for it in the specified group, if not found, the first contact of group will be displayed

@mention-bot
Copy link

By analyzing the blame information on this pull request, we identified @irgendwie, @tomneedham and @DeepDiver1975 to be potential reviewers

@codecov-io
Copy link

Current coverage is 10.91%

Merging #323 into master will decrease coverage by -0.11% as of 99f45e4

@@            master    #323   diff @@
======================================
  Files           41      41       
  Stmts          744     751     +7
  Branches         0       0       
  Methods          0       0       
======================================
  Hit             82      82       
  Partial          0       0       
- Missed         662     669     +7

Review entire Coverage Diff as of 99f45e4

Uncovered Suggestions

  1. +2.13% via ...ntactColor_filter.js#3...18
  2. +2.00% via ...dressBook_service.js#148...162
  3. +1.60% via ...ntactColor_filter.js#23...34
  4. See 7 more...

Powered by Codecov. Updated on successful CI builds.

@DeepDiver1975 DeepDiver1975 added this to the 1.2 milestone Mar 31, 2016
@skjnldsv skjnldsv changed the title First contact load fix, group load and order fix #312 First contact load fix,direct access to contact, contact not in group, group load and order fix #312 Mar 31, 2016
@skjnldsv skjnldsv mentioned this pull request Mar 31, 2016
Closed
@skjnldsv
Copy link
Contributor Author

@Henni :)

@skjnldsv
Copy link
Contributor Author

Rebased with small fix.

@skjnldsv
Copy link
Contributor Author

skjnldsv commented Apr 1, 2016

@jancborchardt a little review here too?

@tomneedham
Copy link
Member

@skjnldsv everyone knows you can only buy PRs with 🍻 ;) I'll do some batch reviewing when I'm back at the desk this evening.

@skjnldsv
Copy link
Contributor Author

skjnldsv commented Apr 1, 2016

Damn, busted! :3

Edit: @tomneedham, well? :D

@skjnldsv
Copy link
Contributor Author

skjnldsv commented Apr 3, 2016

Fixed a small conflict with the group selection on mobile view.

@skjnldsv skjnldsv changed the title First contact load fix,direct access to contact, contact not in group, group load and order fix #312 First contact load fix, direct access to contact, contact not in group, group load and order - fix #312 Apr 3, 2016
@jancborchardt
Copy link
Member

@skjnldsv rebase again :)

@tomneedham you wanna earn your 🍻?

@skjnldsv
Copy link
Contributor Author

skjnldsv commented Apr 5, 2016

Rebased and ready to merge.

Henni
Henni reviewed Apr 5, 2016
View reviewed changes
js/components/contactList/contactList_controller.js
gid: $routeParams.gid,
uid: undefined
});
}
Copy link
Contributor

Choose a reason for hiding this comment

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

why was this else clause removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because it was causing issues. It was replaced by this test:

contacts/js/components/contactList/contactList_controller.js

Line 50 in dabce47

if (ctrl.contactList[i].uid() === ev.uid) {

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@skjnldsv
First contact load fix, group load and order (fix #312)
74ff3b5 
Fixed direct link to contact and invalid contact access in group (fix #318)
Fixed page loading with invalid group
Fix mobile merge and group contact load issue
@skjnldsv skjnldsv merged commit 79a13b8 into master Apr 5, 2016
@skjnldsv skjnldsv deleted the fix-first-contact-load branch April 5, 2016 14:32
@skjnldsv skjnldsv mentioned this pull request Apr 7, 2016
Merged
@MariusBluem MariusBluem mentioned this pull request Feb 13, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
4 - to release
Projects
None yet
Milestone
1.2
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@skjnldsv @mention-bot @codecov-io @tomneedham @jancborchardt @Henni @DeepDiver1975