Added exit signs to the instruction banner and refactored instruction loader #1195
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| package com.mapbox.services.android.navigation.ui.v5.instruction; | ||
|
|
||
| import com.mapbox.api.directions.v5.models.BannerComponents; | ||
| import com.mapbox.core.utils.TextUtils; | ||
|
|
||
| class AbbreviationVerifier extends NodeVerifier { | ||
| @Override | ||
| boolean isNodeType(BannerComponents bannerComponents) { | ||
| return hasAbbreviation(bannerComponents); | ||
| } | ||
|
|
||
| private boolean hasAbbreviation(BannerComponents components) { | ||
| return !TextUtils.isEmpty(components.abbreviation()); | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| package com.mapbox.services.android.navigation.ui.v5.instruction; | ||
|
|
||
| import com.mapbox.api.directions.v5.models.BannerComponents; | ||
|
|
||
| /** | ||
| * Class used to construct a list of BannerComponents to be populated into a TextView | ||
| */ | ||
| class BannerComponentNode { | ||
| BannerComponents bannerComponents; | ||
| int startIndex; | ||
|
|
||
| BannerComponentNode(BannerComponents bannerComponents, int startIndex) { | ||
| this.bannerComponents = bannerComponents; | ||
| this.startIndex = startIndex; | ||
| } | ||
|
|
||
| @Override | ||
| public String toString() { | ||
| return bannerComponents.text(); | ||
| } | ||
|
|
||
| void setStartIndex(int startIndex) { | ||
| this.startIndex = startIndex; | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| package com.mapbox.services.android.navigation.ui.v5.instruction; | ||
|
|
||
| import android.support.annotation.NonNull; | ||
| import android.widget.TextView; | ||
|
|
||
| import com.mapbox.api.directions.v5.models.BannerComponents; | ||
| import com.mapbox.api.directions.v5.models.BannerText; | ||
|
|
||
| import java.util.ArrayList; | ||
| import java.util.List; | ||
|
|
||
| class BannerComponentTree { | ||
| private final NodeCreator[] nodeCreators; | ||
| private final List<BannerComponentNode> bannerComponentNodes; | ||
|
|
||
| /** | ||
| * Creates a master coordinator to make sure the coordinators passed in are used appropriately | ||
| * | ||
| * @param nodeCreators coordinators in the order that they should process banner components | ||
| */ | ||
| BannerComponentTree(@NonNull BannerText bannerText, NodeCreator... nodeCreators) { | ||
| this.nodeCreators = nodeCreators; | ||
| bannerComponentNodes = parseBannerComponents(bannerText); | ||
| } | ||
|
|
||
| /** | ||
| * Parses the banner components and processes them using the nodeCreators in the order they | ||
| * were originally passed | ||
| * | ||
| * @param bannerText to parse | ||
| * @return the list of nodes representing the bannerComponents | ||
| */ | ||
| private List<BannerComponentNode> parseBannerComponents(BannerText bannerText) { | ||
| int length = 0; | ||
| List<BannerComponentNode> bannerComponentNodes = new ArrayList<>(); | ||
|
|
||
| for (BannerComponents components : bannerText.components()) { | ||
| BannerComponentNode node = null; | ||
| for (NodeCreator nodeCreator : nodeCreators) { | ||
| if (nodeCreator.isNodeType(components)) { | ||
| node = nodeCreator.setupNode(components, bannerComponentNodes.size(), length, | ||
| bannerText.modifier()); | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| if (node != null) { | ||
| bannerComponentNodes.add(node); | ||
| length += components.text().length(); | ||
| } | ||
| } | ||
|
|
||
| return bannerComponentNodes; | ||
| } | ||
|
|
||
| /** | ||
| * Loads the instruction into the given text view. If things have to be done in a particular order, | ||
| * the coordinator methods preProcess and postProcess can be used. PreProcess should be used to | ||
| * load text into the textView (so there should only be one coordinator calling this method), and | ||
| * postProcess should be used to make changes to that text, i.e., to load images into the textView. | ||
| * | ||
| * @param textView in which to load text and images | ||
| */ | ||
| void loadInstruction(TextView textView) { | ||
| for (NodeCreator nodeCreator : nodeCreators) { | ||
| nodeCreator.preProcess(textView, bannerComponentNodes); | ||
| } | ||
|
|
||
| for (NodeCreator nodeCreator : nodeCreators) { | ||
| nodeCreator.postProcess(textView, bannerComponentNodes); | ||
| } | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| package com.mapbox.services.android.navigation.ui.v5.instruction; | ||
|
|
||
| import android.content.Context; | ||
| import android.view.LayoutInflater; | ||
| import android.view.ViewGroup; | ||
| import android.widget.TextView; | ||
|
|
||
| import com.mapbox.api.directions.v5.models.BannerComponents; | ||
| import com.mapbox.services.android.navigation.ui.v5.R; | ||
|
|
||
| import java.util.List; | ||
|
|
||
| class ExitSignCreator extends NodeCreator<BannerComponentNode, ExitSignVerifier> { | ||
| private String exitNumber; | ||
| private int startIndex; | ||
| private TextViewUtils textViewUtils; | ||
| private String modifier; | ||
|
|
||
| ExitSignCreator() { | ||
| super(new ExitSignVerifier()); | ||
| textViewUtils = new TextViewUtils(); | ||
| } | ||
|
|
||
| @Override | ||
| BannerComponentNode setupNode(BannerComponents components, int index, int startIndex, String | ||
| modifier) { | ||
| if (components.type().equals("exit")) { | ||
|
||
| return null; | ||
|
Contributor
There was a problem hiding this comment. Why do we return
Contributor
Author
There was a problem hiding this comment. @danesfeder this contains the text in the native language representing "exit", but for parity with iOS we are dropping this node (See screenshots, it doesn't say |
||
| } else if (components.type().equals("exit-number")) { | ||
| exitNumber = components.text(); | ||
| this.startIndex = startIndex; | ||
| this.modifier = modifier; | ||
| } | ||
|
|
||
| return new BannerComponentNode(components, startIndex); | ||
| } | ||
|
|
||
| /** | ||
| * One coordinator should override this method, and this should be the coordinator which populates | ||
| * the textView with text. | ||
| * | ||
| * @param textView to populate | ||
| * @param bannerComponentNodes containing instructions | ||
| */ | ||
| @Override | ||
| void postProcess(TextView textView, List<BannerComponentNode> bannerComponentNodes) { | ||
| if (exitNumber != null) { | ||
| LayoutInflater inflater = (LayoutInflater) textView.getContext().getSystemService(Context | ||
| .LAYOUT_INFLATER_SERVICE); | ||
|
|
||
| ViewGroup root = (ViewGroup) textView.getParent(); | ||
|
|
||
| TextView exitSignView; | ||
|
|
||
| if (modifier.equals("left")) { | ||
| exitSignView = (TextView) inflater.inflate(R.layout.exit_sign_view_left, root, false); | ||
| } else { | ||
| exitSignView = (TextView) inflater.inflate(R.layout.exit_sign_view_right, root, false); | ||
| } | ||
|
|
||
| exitSignView.setText(exitNumber); | ||
|
|
||
| textViewUtils.setImageSpan(textView, exitSignView, startIndex, startIndex + exitNumber | ||
| .length()); | ||
| } | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| package com.mapbox.services.android.navigation.ui.v5.instruction; | ||
|
|
||
| import com.mapbox.api.directions.v5.models.BannerComponents; | ||
|
|
||
| class ExitSignVerifier extends NodeVerifier { | ||
|
|
||
| @Override | ||
| boolean isNodeType(BannerComponents bannerComponents) { | ||
| return bannerComponents.type().equals("exit") || bannerComponents.type().equals("exit-number"); | ||
| } | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed internally, should we remove this constructor? IMO it's not adding any value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Guardiola31337 What discussion? Are you talking about the discussion about Dan's PR? If you are, you said that you didn't feel strongly about that either way. I think this is adding the value of not making the
InstructionLoaderinstantiate the hashmap and text utils while still making this testable. It's basically acting as a dependency injection utility method