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

Fix: Declare NewsBrainzActivity in androidManifest #160

Conversation

prabalsingh24
Copy link
Contributor

Clicking the News button in the main screen does not open the News activity as it's not defined in the AndroidManifest

Clicking the News button in the main screen does not open
the News activity as it's not defined in the AndroidManifest
@prabalsingh24
Copy link
Contributor Author

@akshaaatt idk why but I can't seem to add you a reviewer. Tagging you :)

Copy link
Member

@akshaaatt akshaaatt left a comment

Choose a reason for hiding this comment

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

Thanks a lot for bringing this up! 💯
I removed it by mistake in my previous commits while removing the Settings activity.

android:exported="false"
android:label="@string/title_activity_news_brainz"
android:theme="@style/AppTheme">
<meta-data
Copy link
Member

Choose a reason for hiding this comment

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

This is not a shared library or anything so we don't need this tag here. Please remove it.

@@ -38,6 +38,16 @@
android:value="" />
</activity>

<activity
android:name=".ui.screens.newsbrainz.NewsBrainzActivity"
android:exported="false"
Copy link
Member

Choose a reason for hiding this comment

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

We don't actually need to specify this until and unless an intent filter is added to the activity.

android:name=".ui.screens.newsbrainz.NewsBrainzActivity"
android:exported="false"
android:label="@string/title_activity_news_brainz"
android:theme="@style/AppTheme">
Copy link
Member

Choose a reason for hiding this comment

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

The default theme for the app is this, hence even if we don't duplicate it here, it will pick the same theme.

@akshaaatt
Copy link
Member

@prabalsingh24 I was in a hurry to make a beta release of the app, hence pushed the fix for it. Thanks a lot for bringing this up though! 💯

@akshaaatt akshaaatt merged commit bc07a9c into metabrainz:main May 29, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants