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

[Palette list] Adapt the palette list to the kids/adult falvors. #18

Merged
merged 1 commit into from Jul 24, 2016

Conversation

vbarthel-fr
Copy link
Member

This MR adapt the palette list for kids.

In the kids version of the app, the palettes are now displayed in a grid. The names of the palette are no longer shown.

Here is a screenshot of the default version:
device-2016-07-24-150847

Here is a screenshot of the kids version:
device-2016-07-24-150915

@tbarthel-fr Compared to my last MR for the color items, I managed to share more code between product flavors. If this is OK for you, I will probably factorize the code for the color items =)

The PaletteListWrapper describes the common contract of the wrapper.
The FlavorPaletteListWrapper defines the specific behaviors.

For the kids flavor, the palettes are presented in a grid.
Only a preview of the palette is presented.

For the adult flavor, the palettes are presented in a vertical list.
The name of the palette is displayed next to the preview.
@jo-elimu jo-elimu merged commit aacfc8c into kid-matters Jul 24, 2016
@jo-elimu
Copy link
Member

By the way, when the palette list is empty, it says "To add colors to your palette simply click on the list below."

@vbarthel-fr
Copy link
Member Author

vbarthel-fr commented Jul 24, 2016

@jogrimst You have just merged an MR that was in the review process (labelled 'to-review' and assigned to Thomas). I don't know if Thomas has explained you how we would like to work on this project, but here is a short summary:

For consistency accross the project, for keeping a global vison of the project, and for double checking code for bugs, the MRs are always reviewed by another peer. The submitter lables the MR with 'to-review' and assigns another member to review his/her work. When the assignee is done, he/she labels the MR with 'reviewed' and assigns the original submitter.

When the code has been properly reviewed, we include you in the loop with the label 'to-validate'. You can then make us your comments/remarks. When you are satisfied with the feature, you should label the MR with 'validated' and assignes the original submitter.

At that point the MR has been reviewed by another developer and validated by you, it should have two labels 'reviewed' and 'validated'. And should be assigned to the original submitter.

The original submitter can now check if there are no merge conflicts with the base and merge!

It might seems a little bit overkilled for such a project, but we hope that it will help us keep this project in good shape and well organized =)

That being said, this flow is certainly not written in stone: please do not hesitate to tell us if you would like to use another work-flow :)

@jo-elimu
Copy link
Member

@vbarthel-fr Thanks for clarifying :-)

I think it's great that you are doing it this way, even for a small project. Such a work-flow keeps the quality high.

A question: Is there any way to test the changes in a pull request on an Android device before it's merged into the branch? It can sometimes be difficult to know how animations work just by looking at code changes without actually navigating the interface.

@vbarthel-fr
Copy link
Member Author

@jogrimst You can checkout the source branch and build the project from that branch. In this case, you could have pulled he branch vb/palette-list to test the new palette list =)

@tbarthel-fr I am going to delete the source branch of this MR and mark it as "reviewed" and "validated". If you find something that requires a fix or a modification, please open an issue and assign me =)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants