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.

Sign up for GitHub

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

(fixes #2872) Refactoring unused files for "showcase view whole nearby activity" #5870

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

(fixes #2872) Refactoring unused files for "showcase view whole nearby activity" #5870

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6302e17
Added consistency to capitalisation
devbytespark Oct 19, 2024
a960f18
Tried removing the button string
devbytespark Oct 19, 2024
df985e1
localisation: deleted instances of refactored string and removed unus…
devbytespark Oct 20, 2024
cb20b56
refactor: Deleted unused dialog_nearby.xml file and its associated st…
devbytespark Oct 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions app/src/main/res/layout/fragment_explore_map.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
android:padding="@dimen/activity_margin_horizontal"
android:singleLine="true"
android:text="@string/search_this_area"
android:textAllCaps="false"
Copy link
Member

Choose a reason for hiding this comment

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

What is the problem if you don't add this line?

Copy link
Author

Choose a reason for hiding this comment

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

The line was initially included to ensure that the button text aligned with the original TextView string (which has now been removed) as Android Studio defaults buttons to AllCaps. However, since we are removing it, it may no longer be necessary.

Copy link
Member

Choose a reason for hiding this comment

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

How about removing this whole button?

Copy link
Author

Choose a reason for hiding this comment

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

While the TextView that used to prompt users to use this button has been refactored and removed, the button remains in the app and is still used in the Map Explore activity, as shown below button_usage

Copy link
Member

Choose a reason for hiding this comment

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

Do you know what makes the text become uppercase in the first place?
We should fix the root issue rather than address this single symptom.
Would you mind checking whether these are really needed?

app/src//main/res/layout-land/activity_review.xml:        android:textAllCaps="true"
app/src//main/res/layout/bottom_sheet_details_explore.xml:            android:textAllCaps="true"
app/src//main/res/layout/bottom_sheet_details_explore.xml:            android:textAllCaps="true"
app/src//main/res/layout/bottom_sheet_details_explore.xml:          android:textAllCaps="true"
app/src//main/res/layout/fragment_achievements.xml:            android:textAllCaps="true"/>
app/src//main/res/layout/fragment_achievements.xml:            android:textAllCaps="true"/>
app/src//main/res/layout/custom_nearby_tab_layout.xml:        android:textAllCaps="true"
app/src//main/res/layout/activity_review.xml:      android:textAllCaps="true"
app/src//main/res/layout/fragment_review_image.xml:      android:textAllCaps="true"
app/src//main/res/layout/fragment_review_image.xml:      android:textAllCaps="true"

android:textColor="@color/status_bar_blue"
android:visibility="gone"
app:elevation="@dimen/dimen_6" />
Expand Down
2 changes: 1 addition & 1 deletion app/src/main/res/values/strings.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@

<string name="about_translate_cancel">Cancel</string>
<string name="retry">Retry</string>
<string name="showcase_view_whole_nearby_activity">These are the places near you that need pictures to illustrate their Wikipedia articles.\n\nClicking on \'SEARCH THIS AREA\' locks the map and launches a nearby search around that location.</string>
<string name="showcase_view_whole_nearby_activity">These are the places near you that need pictures to illustrate their Wikipedia articles.\n\nClicking on \'Search this area\' locks the map and launches a nearby search around that location.</string>
Copy link
Member

Choose a reason for hiding this comment

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

There is no "Search this area" button anymore.
Actually, is this string used somewhere? If you find where it is used, would you mind posting a screenshot?
Thanks a lot! 🙂

Copy link
Author

Choose a reason for hiding this comment

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

I found that it was used in dialog_nearby.xml, but I believe the activity using the layout has been refactored.

Copy link
Member

Choose a reason for hiding this comment

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

Would you mind trying to remove it? Any impact? 🙂

Copy link
Author

@devbytespark devbytespark Oct 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Deleting the button and its string seems fine

Copy link
Author

@devbytespark devbytespark Oct 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Hey Nicholas after further review I noticed that the dialog_nearby.xml file had no usages in the whole app. I’ve safely deleted the file along with its associated string resources, and the project still builds without issues.

It seems the file was part of the old webview in the nearby section of the contributions activity, likely from a version of the app prior to refactoring.

<string name="showcase_view_needs_photo">This place needs a photo.</string>
<string name="showcase_view_has_photo">This place already has a photo.</string>
<string name="showcase_view_no_longer_exists">This place no longer exists.</string>
Expand Down