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

Add option to create a new note everywhere #732

Merged
merged 9 commits into from Jan 17, 2018
Expand Up @@ -22,6 +22,8 @@ public class ApplicationConstants

public final static int QUEST_TILE_ZOOM = 14;

public final static int NOTE_MIN_ZOOM = 15;

/** a "best before" duration for quests. Quests will not be downloaded again for any tile
* before the time expired */
public static final int REFRESH_QUESTS_AFTER = 7*24*60*60*1000; // one week in ms
Expand Down
33 changes: 33 additions & 0 deletions app/src/main/java/de/westnordost/streetcomplete/MainActivity.java
Expand Up @@ -58,6 +58,8 @@
import de.westnordost.streetcomplete.data.VisibleQuestListener;
import de.westnordost.streetcomplete.data.osm.OsmQuest;
import de.westnordost.streetcomplete.data.upload.VersionBannedException;
import de.westnordost.streetcomplete.data.osmnotes.CreateNoteDialog;
import de.westnordost.streetcomplete.data.osmnotes.CreateNoteFragment;
import de.westnordost.streetcomplete.location.LocationRequestFragment;
import de.westnordost.streetcomplete.location.LocationUtil;
import de.westnordost.streetcomplete.oauth.OAuthPrefs;
Expand Down Expand Up @@ -344,11 +346,36 @@ private int getQuestTitleResId(Quest quest, Map<String,String> tags)
if(isConnected()) uploadChanges();
else Toast.makeText(this, R.string.offline, Toast.LENGTH_SHORT).show();
return true;
case R.id.action_note:
createNote();
return true;
}

return super.onOptionsItemSelected(item);
}

@UiThread private void createNote()
{
if (mapFragment.getZoom() < ApplicationConstants.NOTE_MIN_ZOOM)
{
Toast.makeText(this, R.string.create_new_note_unprecise, Toast.LENGTH_LONG).show();
}
else
{
CreateNoteFragment f = new CreateNoteFragment();
Bundle args = new Bundle();
LatLon pos = mapFragment.getPosition();
args.putDouble(CreateNoteDialog.ARG_LAT, pos.getLatitude());
args.putDouble(CreateNoteDialog.ARG_LON, pos.getLongitude());
Copy link
Member

Choose a reason for hiding this comment

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

You use CreateNoteFragment but use the constants from CreateNoteDialog. That looks fishy.

Copy link
Member

Choose a reason for hiding this comment

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

Nevermind that comment (can't remove it on mobile)

f.setArguments(args);

FragmentTransaction ft = getSupportFragmentManager().beginTransaction();
ft.add(R.id.map_create_note_container, f, CREATE_NOTE);
Copy link
Member

Choose a reason for hiding this comment

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

Couldn't the normal bottom sheet container be used? (Not sure right now, just looking at the diff)

ft.addToBackStack(CREATE_NOTE);
ft.commit();
}
}

@UiThread private void uploadChanges()
{
// because the app should ask for permission even if there is nothing to upload right now
Expand Down Expand Up @@ -559,6 +586,7 @@ else if(e instanceof OsmAuthorizationException)
/* ------------ Managing bottom sheet (quest details) and interaction with map ------------- */

private final static String BOTTOM_SHEET = "bottom_sheet";
public final static String CREATE_NOTE = "create_note";

@Override public void onBackPressed()
{
Expand Down Expand Up @@ -597,6 +625,11 @@ else if(e instanceof OsmAuthorizationException)
questController.createNote(questId, questTitle, note, imagePaths);
}

@Override public void onLeaveNote(String note, ArrayList<String> imagePaths, LatLon position)
{
questController.createNote(note, imagePaths, position);
}

@Override public void onSkippedQuest(long questId, QuestGroup group)
{
closeQuestDetailsFor(questId, group);
Expand Down
Expand Up @@ -17,6 +17,7 @@
import javax.inject.Inject;
import javax.inject.Provider;

import de.westnordost.osmapi.map.data.LatLon;
import de.westnordost.streetcomplete.ApplicationConstants;
import de.westnordost.streetcomplete.data.changesets.OpenChangesetsDao;
import de.westnordost.streetcomplete.data.download.QuestDownloadService;
Expand Down Expand Up @@ -182,6 +183,18 @@ public void createNote(final long osmQuestId, final String questTitle, final Str
});
}

public void createNote(final String text, final ArrayList<String> imagePaths, LatLon position)
{
workerHandler.post(() ->
{
CreateNote createNote = new CreateNote();
createNote.position = position;
createNote.text = text;
createNote.imagePaths = imagePaths;
createNoteDB.add(createNote);
});
}

/** Apply the user's answer to the given quest. (The quest will turn invisible.) */
public void solveQuest(final long questId, final QuestGroup group, final Bundle answer,
final String source)
Expand Down
Expand Up @@ -46,7 +46,10 @@ public boolean add(CreateNote note)
values.put(CreateNoteTable.Columns.IMAGE_PATHS, serializer.toBytes(note.imagePaths));
}
values.put(CreateNoteTable.Columns.TEXT, note.text);
values.put(CreateNoteTable.Columns.QUEST_TITLE, note.questTitle);
if (note.questTitle != null)
{
values.put(CreateNoteTable.Columns.QUEST_TITLE, note.questTitle);
Copy link
Member

Choose a reason for hiding this comment

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

Does the table allow NULL there?

Copy link
Member

Choose a reason for hiding this comment

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

Also, could questTitle being null cause a NPE someplace else?

Copy link
Member

Choose a reason for hiding this comment

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

Ok nevermind this comment.

Copy link
Member

Choose a reason for hiding this comment

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

Whoops, I meant to write that someplace else, do mind that comment!

Copy link
Contributor

Choose a reason for hiding this comment

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

(BTW: You can delete comments…)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does the table allow NULL there?

I think so... In the StreetCompleteOpenHelper it does not say NOT NULL

Also, could questTitle being null cause a NPE someplace else?

I don't think so... As far as I have seen, there are null-checks everywhere.

}

long rowId = db.insert(CreateNoteTable.NAME, null, values);

Expand Down
@@ -0,0 +1,89 @@
package de.westnordost.streetcomplete.data.osmnotes;

import android.os.Bundle;
import android.support.annotation.Nullable;
import android.support.v4.app.DialogFragment;
import android.view.LayoutInflater;
import android.view.View;
import android.view.ViewGroup;
import android.widget.EditText;
import android.widget.Toast;

import de.westnordost.osmapi.map.data.LatLon;
import de.westnordost.osmapi.map.data.OsmLatLon;
import de.westnordost.streetcomplete.R;
import de.westnordost.streetcomplete.quests.QuestAnswerComponent;
import de.westnordost.streetcomplete.quests.note_discussion.AttachPhotoFragment;


public class CreateNoteDialog extends DialogFragment
{

private EditText noteInput;

private QuestAnswerComponent questAnswerComponent;

public static final String ARG_LAT = "latitude";
public static final String ARG_LON = "longitude";

public CreateNoteDialog()
{
super();
questAnswerComponent = new QuestAnswerComponent();
}

@Override
public View onCreateView(LayoutInflater inflater, ViewGroup container,
Bundle savedInstanceState)
{
View view = inflater.inflate(R.layout.dialog_create_note, container, false);

view.findViewById(R.id.buttonCancel).setOnClickListener(v -> onClickCancel());
view.findViewById(R.id.buttonOk).setOnClickListener(v -> onClickOk());

noteInput = view.findViewById(R.id.noteInput);

return view;
}

@Override public void onViewCreated(View view, @Nullable Bundle savedInstanceState)
{
super.onViewCreated(view, savedInstanceState);
if(savedInstanceState == null)
{
getChildFragmentManager().beginTransaction().add(R.id.attachPhotoFragment, new AttachPhotoFragment()).commit();
}
}

private @Nullable AttachPhotoFragment getAttachPhotoFragment()
{
return (AttachPhotoFragment) getChildFragmentManager().findFragmentById(R.id.attachPhotoFragment);
}

private void onClickOk()
{
String noteText = noteInput.getText().toString().trim();

if(noteText.isEmpty())
{
Toast.makeText(getActivity(), R.string.no_changes, Toast.LENGTH_SHORT).show();
return;
}

AttachPhotoFragment f = getAttachPhotoFragment();

Double latitude = getArguments().getDouble(ARG_LAT);
Double longitude = getArguments().getDouble(ARG_LON);
LatLon position = new OsmLatLon(latitude, longitude);

questAnswerComponent.onLeaveNote(noteText, f != null ? f.getImagePaths() : null, position);
dismiss();
}

private void onClickCancel()
{
AttachPhotoFragment f = getAttachPhotoFragment();
if(f != null) f.deleteImages();
dismiss();
}
}
@@ -0,0 +1,53 @@
package de.westnordost.streetcomplete.data.osmnotes;

import android.os.Bundle;
import android.support.v4.app.DialogFragment;
import android.support.v4.app.Fragment;
import android.support.v4.app.FragmentManager;
import android.support.v7.app.AppCompatActivity;
import android.support.v7.widget.Toolbar;
import android.view.LayoutInflater;
import android.view.View;
import android.view.ViewGroup;
import android.widget.ImageButton;

import de.westnordost.streetcomplete.MainActivity;
import de.westnordost.streetcomplete.R;

public class CreateNoteFragment extends Fragment
{

@Override
public View onCreateView(LayoutInflater inflater, ViewGroup container,
Bundle savedInstanceState)
{
View view = inflater.inflate(R.layout.fragment_create_note, container, false);

Toolbar noteToolbar = view.findViewById(R.id.note_toolbar);
((AppCompatActivity)getActivity()).setSupportActionBar(noteToolbar);

ImageButton finishNoteCreation = view.findViewById(R.id.finish_note_creation);
ImageButton closeNoteCreation = view.findViewById(R.id.close_note_creation);

finishNoteCreation.setOnClickListener((v) -> openDialog());

closeNoteCreation.setOnClickListener((v) ->
{
getActivity().getSupportFragmentManager().popBackStackImmediate(MainActivity.CREATE_NOTE, FragmentManager.POP_BACK_STACK_INCLUSIVE);
});

return view;
}

private void openDialog()
{
DialogFragment createNoteDialog = new CreateNoteDialog();

Bundle createNoteArgs = new Bundle();
createNoteArgs.putDouble(CreateNoteDialog.ARG_LAT, getArguments().getDouble(CreateNoteDialog.ARG_LAT));
createNoteArgs.putDouble(CreateNoteDialog.ARG_LON, getArguments().getDouble(CreateNoteDialog.ARG_LAT));
createNoteDialog.setArguments(createNoteArgs);

createNoteDialog.show(getActivity().getSupportFragmentManager(), null);
}
}
Expand Up @@ -4,6 +4,7 @@

import java.util.ArrayList;

import de.westnordost.osmapi.map.data.LatLon;
import de.westnordost.streetcomplete.data.QuestGroup;

public interface OsmQuestAnswerListener
Expand All @@ -17,4 +18,7 @@ public interface OsmQuestAnswerListener

/** Called when the user chose to skip the quest */
void onSkippedQuest(long questId, QuestGroup group);

/** Called when the user wants to leave a note which is not related to a quest */
void onLeaveNote(String note, ArrayList<String> imagePaths, LatLon position);
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, as the callback doesn't have anything to do with quests (or answering them), the method should be in an own interface and not in this.

}
Expand Up @@ -4,6 +4,7 @@

import java.util.ArrayList;

import de.westnordost.osmapi.map.data.LatLon;
import de.westnordost.streetcomplete.data.QuestGroup;

public class QuestAnswerComponent
Expand Down Expand Up @@ -57,6 +58,11 @@ public void onLeaveNote(String questTitle, String text, ArrayList<String> imageP
callbackListener.onLeaveNote(questId, questGroup, questTitle, text, imagePaths);
}

public void onLeaveNote(String text, ArrayList<String> imagePaths, LatLon position)
{
callbackListener.onLeaveNote(text, imagePaths, position);
}

Copy link
Member

Choose a reason for hiding this comment

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

Should also not be here. Has nothing to do with quests.

public void onSkippedQuest()
{
callbackListener.onSkippedQuest(questId, questGroup);
Expand Down
Expand Up @@ -16,6 +16,7 @@
import android.support.annotation.Nullable;
import android.support.v13.app.FragmentCompat;
import android.support.v4.app.Fragment;
import android.support.v4.app.FragmentTransaction;
import android.support.v7.preference.PreferenceManager;
import android.text.Html;
import android.text.TextUtils;
Expand All @@ -27,6 +28,7 @@
import android.view.ViewTreeObserver;
import android.view.WindowManager;
import android.widget.TextView;
import android.widget.Toast;

import com.mapzen.android.lost.api.LocationListener;
import com.mapzen.android.lost.api.LocationRequest;
Expand All @@ -43,17 +45,21 @@
import java.io.File;

import de.westnordost.osmapi.map.data.LatLon;
import de.westnordost.streetcomplete.ApplicationConstants;
import de.westnordost.streetcomplete.Prefs;
import de.westnordost.streetcomplete.R;
import de.westnordost.streetcomplete.data.osmnotes.CreateNoteDialog;
import de.westnordost.streetcomplete.data.osmnotes.CreateNoteFragment;
import de.westnordost.streetcomplete.util.SphericalEarthMath;

import static android.content.Context.SENSOR_SERVICE;
import static de.westnordost.streetcomplete.MainActivity.CREATE_NOTE;

public class MapFragment extends Fragment implements
FragmentCompat.OnRequestPermissionsResultCallback, LocationListener,
LostApiClient.ConnectionCallbacks, TouchInput.ScaleResponder,
TouchInput.ShoveResponder, TouchInput.RotateResponder,
TouchInput.PanResponder, TouchInput.DoubleTapResponder, CompassComponent.Listener, MapController.SceneLoadListener
TouchInput.PanResponder, TouchInput.DoubleTapResponder, TouchInput.LongPressResponder, CompassComponent.Listener, MapController.SceneLoadListener
{
private CompassComponent compass = new CompassComponent();

Expand Down Expand Up @@ -138,6 +144,7 @@ public void getMapAsync(String apiKey)
controller.setScaleResponder(this);
controller.setPanResponder(this);
controller.setDoubleTapResponder(this);
controller.setLongPressResponder(this);
updateMapTileCacheSize();
controller.setHttpHandler(httpHandler);

Expand Down Expand Up @@ -375,6 +382,28 @@ protected void followPosition()
return false;
}

@Override public void onLongPress(float x, float y)
{
if (controller.getZoom() < ApplicationConstants.NOTE_MIN_ZOOM)
{
Toast.makeText(getActivity(), R.string.create_new_note_unprecise, Toast.LENGTH_LONG).show();
}
else
{
CreateNoteFragment f = new CreateNoteFragment();
Bundle args = new Bundle();
LngLat pos = controller.getPosition();
args.putDouble(CreateNoteDialog.ARG_LAT, pos.latitude);
args.putDouble(CreateNoteDialog.ARG_LON, pos.longitude);
f.setArguments(args);

FragmentTransaction ft = getActivity().getSupportFragmentManager().beginTransaction();
ft.add(R.id.map_create_note_container, f, CREATE_NOTE);
ft.addToBackStack(CREATE_NOTE);
ft.commit();
}
}

private void onMapOrientation()
{
onMapOrientation(controller.getRotation(), controller.getTilt());
Expand Down
Expand Up @@ -478,4 +478,9 @@ public LatLon getPosition()
return TangramConst.toLatLon(controller.getPosition());
}

public float getZoom()
{
return controller.getZoom();
}

Copy link
Member

Choose a reason for hiding this comment

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

Should perhaps go into MapsFragment as it has nothing to do with quests?

}
9 changes: 9 additions & 0 deletions app/src/main/res/drawable/ic_check_white_24dp.xml
@@ -0,0 +1,9 @@
<vector xmlns:android="http://schemas.android.com/apk/res/android"
android:width="24dp"
android:height="24dp"
android:viewportWidth="24.0"
android:viewportHeight="24.0">
<path
android:fillColor="#FFFFFFFF"
android:pathData="M9,16.17L4.83,12l-1.42,1.41L9,19 21,7l-1.41,-1.41z"/>
</vector>