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

Jump to bottom

[RN, Android] Suggestions #318

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[RN, Android] Suggestions #318

wants to merge 1 commit into from

Conversation

peterBrxwn
Copy link
Contributor

No description provided.

turtledreams
turtledreams reviewed Mar 24, 2023
View reviewed changes
android/next.md
@@ -714,6 +714,8 @@ Type idType = Countly.sharedInstance().deviceId().getType();</code></pre>
<p>Select Cloud Messaging tab</p>
<div class="img-container">
<img src="https://count.ly/images/guide/fb244d1-Screenshot-2018-04-21-17.20.41-x.png">
<!-- Above image is out of date. For new firebase accounts, Firebase Cloud Messaging API (Legacy) is Disabled -->
Copy link
Contributor

Choose a reason for hiding this comment

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

FAIR POINNT~!

Copy link
Contributor

Choose a reason for hiding this comment

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

image

Copy link
Contributor

Choose a reason for hiding this comment

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

image
OLD

turtledreams
turtledreams reviewed Mar 24, 2023
View reviewed changes
android/next.md
@@ -735,6 +737,7 @@ Type idType = Countly.sharedInstance().deviceId().getType();</code></pre>
<span style="font-weight: 400;">Add the following dependency to your <code>build.gradle</code></span><span style="font-weight: 400;">&nbsp;(</span><strong>use latest Firebase version</strong><span style="font-weight: 400;">):</span>
</p>
<pre>//latest firebase-messaging version that is available<code class="java">
<!-- Link to latest version: https://mvnrepository.com/artifact/com.google.firebase/firebase-messaging -->
Copy link
Contributor

Choose a reason for hiding this comment

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

FAIR POINT NEW VERSION IS 23.1.2
mb give thi slink

turtledreams
turtledreams reviewed Mar 24, 2023
View reviewed changes
android/next.md
@@ -746,6 +749,7 @@ implementation 'com.google.firebase:firebase-messaging:22.0.0'</code></pre>
&lt;/service&gt;
</code></pre>
<p>... and add a class for it as well:</p>
<!-- Can we let flutter users know that we should skip this or add "not needed for flutter projects". Because I tried to implement it. -->
Copy link
Contributor

Choose a reason for hiding this comment

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

THERE IS A LINK TO THIS AT FLUTTER AND RN BUT SEEMS TO BE UNNCESSARY, INVESTIGATE

Copy link
Member

Choose a reason for hiding this comment

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

we should mention that the service files are added implicitly by the SDK

turtledreams
turtledreams reviewed Mar 24, 2023
View reviewed changes
flutter/next.md
</intent-filter>
</service>
</application>
-->
Copy link
Contributor

Choose a reason for hiding this comment

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

SUGGESTION TO GIVE EXACT LOCATION WHERE TO ADD THIS CODE as it was ambiguous

turtledreams
turtledreams reviewed Mar 24, 2023
View reviewed changes
flutter/next.md
@@ -587,25 +597,30 @@ Countly.recordView("Dashboard");</code></pre>
&lt;/service&gt;
</code></pre>
<p>
<!-- should be step 6 -->
Copy link
Contributor

Choose a reason for hiding this comment

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

FAIR POINT STEP 5 to 7. THERE IS NOR STEP 6

turtledreams
turtledreams reviewed Mar 24, 2023
View reviewed changes
flutter/next.md
Step 7: Add the following line in file <code>android/app/build.gradle</code>
</p>
<pre><code class="JavaScript">dependencies {
implementation 'ly.count.android:sdk:20.04'
implementation 'ly.count.android:sdk:20.04' <!-- update to 22.02.1 -->
<!-- Link to latest version: https://mvnrepository.com/artifact/com.google.firebase/firebase-messaging -->
Copy link
Contributor

Choose a reason for hiding this comment

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

yea all steps are messed up 5 >7>6>7

turtledreams
turtledreams reviewed Mar 24, 2023
View reviewed changes
flutter/next.md
<p>
For Flutter you can find <code>CountlyNotificationService.h/m</code> file under
<code>Pods/Development Pods/Countly/{PROJECT_NAME}/ios/.symlinks/plugins/countly_flutter/ios/Classes/CountlyiOS/CountlyNotificationService.h/m</code><br>
<br>
<!-- above line is too long. breaks the screen -->
<!-- Also, couldn't find this path "Pods/Development Pods" any where -->
Copy link
Contributor

Choose a reason for hiding this comment

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

ignoreable

turtledreams
turtledreams reviewed Mar 24, 2023
View reviewed changes
flutter/next.md
<strong>Pro Tips to find the files from deep hierarchy:<br></strong>
</p>
<ul>
<li>
You can filter the files in the navigator using a shortcut ⌥⌘J (Option-Command-J),
in the filter box type "CountlyNotificationService" and it will show the
related files only.
<!-- Option-Command-J does nothing on my xcode -->
Copy link
Contributor

Choose a reason for hiding this comment

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

well if you say so

turtledreams
turtledreams reviewed Mar 24, 2023
View reviewed changes
flutter/next.md
</li>
<li>
You can find the file using the shortcut ⇧⌘O (Shift-Command-O) and then navigate
to that file using the shortcut ⇧⌘J (Shift-Command-J)<br>
<!-- Shift-Command-O opens a filter box but the file is not found and Shift-Command-J does not seem to do anything. -->
Copy link
Contributor

Choose a reason for hiding this comment

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

again a mac world

turtledreams
turtledreams reviewed Mar 24, 2023
View reviewed changes
flutter/next.md
@@ -652,6 +672,7 @@ Countly.disablePushNotifications();</code></pre>
</p>
<pre><code class="JavaScript">// Set messaging mode for push notifications
Countly.pushTokenType(Countly.messagingMode["TEST"]);</code></pre>
<!-- Should be Countly.pushTokenType(Countly.messagingMode["TEST"]!); -->
Copy link
Contributor

Choose a reason for hiding this comment

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

if you say so again (~!)

turtledreams
turtledreams reviewed Mar 24, 2023
View reviewed changes
flutter/next.md
@@ -666,6 +687,7 @@ Countly.askForNotificationPermission();</code></pre>
<p>
In order to listen to notification receive and click events, Place below code
in <code>AppDelegate.swift</code>&nbsp;
<!-- when you say "Place below code in AppDelegate.swift", I assume that the "AppDelegate.swift" file is in my root folder or root ios folder. Either I have eye problem or there is no file like that. -->
Copy link
Contributor

Choose a reason for hiding this comment

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

looks like we need a verification for this

turtledreams
turtledreams reviewed Mar 24, 2023
View reviewed changes
flutter/next.md
@@ -622,21 +637,26 @@ Countly.disablePushNotifications();</code></pre>
For iOS push notification please follow the instruction from this URL
<a href="https://support.count.ly/hc/en-us/articles/360037753511-iOS-watchOS-tvOS-macOS#push-notifications">https://resources.count.ly/docs/countly-sdk-for-ios-and-os-x#section-push-notifications</a>
</p>
<!-- For the below file, we are told where to find it, but not what to do with it. Seems like the file is not needed in this page. Rather it is needed in the above link. While on the above link, I could not find the file. -->
Copy link
Contributor

Choose a reason for hiding this comment

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

okay seems like we only said you can find this file here and nothing else

@turtledreams turtledreams added SDK Documentation changes to an SDK's main document Fix Fixing grammar or wrong information labels Apr 5, 2023
@turtledreams turtledreams changed the title suggestions [RN, Android] Suggestions Apr 5, 2023
@ArtursKadikis
Copy link
Member

marked the ones to integrate on the first pass. we'll tackle the others afterwards

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ArtursKadikis ArtursKadikis ArtursKadikis left review comments

@turtledreams turtledreams turtledreams left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
Fix Fixing grammar or wrong information SDK Documentation changes to an SDK's main document
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@peterBrxwn @ArtursKadikis @turtledreams