dorotatomczak / TravelPlannerApp

Travel Planner App
1 stars 0 forks source link

133 share travel #134

Closed ani119 closed 5 years ago

ani119 commented 5 years ago

Add share travel ability. After click on share icon, the dialog appears. It include user friends list without users which already have access to the travel.

Screenshot from 2019-10-19 23-41-07

magasol commented 5 years ago

I would keep edit icon in the end what do you think?

ani119 commented 5 years ago

I would keep edit icon in the end what do you think?

Ok, so you want to have order: picture,share,edit? If it is possible I wanna finish minimum 1 pull request today, tomorrow i must return laptop about 7 am :/ But in the nearest future i will buy extra ram or maybe new laptop

ani119 commented 5 years ago

new version Screenshot from 2019-10-21 00-06-52

anna-malizjusz commented 5 years ago

And generally: Crt+alt+L almost everywhere ;)

magasol commented 5 years ago

but not in xmls ;p

magasol commented 5 years ago

if you could explain the proccess? user clicks on share icon -> the list displayed is with all friends or just the once that doesn;t have access to travel? -> user checks friends, do you send a list of all friends or just the checked once?

ani119 commented 5 years ago

1) User click button share 2) User see dialog with list of emails, which belong to his friends, whos do not have access to selected travel 3) User select some of displayed emails 4) User click ok 5) All of selected users get access to travel

ani119 commented 5 years ago

@magasol @anna-malizjusz @dorotatomczak Could you write me what exaclty I should improve in my code? - all mistakes in list. I want to implement all of your suggestions and finally move on to the next task. As if you could also agree on corrections, because your conflicting suggestions are really confusing. I want to be more engaged to all project, not only two functionalites

dorotatomczak commented 5 years ago

@ani119 I'm waiting till you fix what needs to be fixed in previous PR. @anna-malizjusz and @magasol have left you comments

dorotatomczak commented 5 years ago

I think you can merge it

dorotatomczak commented 5 years ago

if @magasol doesn't have anything against it