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.

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

Move family tree tab to results tabs #280 #359

Merged

Conversation

Inna-r
Copy link
Contributor

@Inna-r Inna-r commented May 4, 2017

  • The changes for general search suggestions added to this PR by mistake were reset.
  • The old person search tab is still alive.
  • The new persons search is provided with its own directive (that works with all 3 types of filters) so it doesn't affect the old version of search.
  • There are some known bugs that i need to fix:
  1. when switching between collection tabs url parameters relevant to persons search have to disappear
  2. Toggling 'show less/more fields' must toggle values inside 'Place of Birth / Marriage / Death' and 'Place of Birth' input fields (if one of these fields is filled)

@Inna-r Inna-r requested a review from OriHoch May 8, 2017 06:48
OriHoch
OriHoch previously requested changes May 8, 2017
Copy link
Contributor

@OriHoch OriHoch left a comment

Choose a reason for hiding this comment

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

code looks good

need to add some documentation

also, following pre-deploy meeting, we will not merge this PR yet

@Inna-r Inna-r requested a review from OriHoch May 8, 2017 12:48
@Inna-r Inna-r dismissed OriHoch’s stale review May 8, 2017 12:49

I updated pull request, please review

Copy link
Contributor

@OriHoch OriHoch left a comment

Choose a reason for hiding this comment

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

approved but please open issues for the known bugs and let's get some feedback from @TheGrandVizier about them

@Inna-r Inna-r merged commit 3d21fd6 into Beit-Hatfutsot:dev May 15, 2017
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.

2 participants