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

[MOB-9995] reorganize source code #599

Merged

Conversation

lposen
Copy link
Contributor

@lposen lposen commented Nov 7, 2024

🔹 JIRA Ticket(s) if any

✏️ Description

Reorganizes source code for better readability
Organizes source code into core, inApp and inbox folders -- eventually, these should be made into packages to reduce bloat in client apps.

lposen and others added 21 commits November 6, 2024 18:24
…d update imports; create new hooks index file
…troduce new enums, and update imports accordingly
…tory, consolidate type imports, and update references accordingly
…ResponseResult, move IterableAttributionInfo and IterableCommerceItem to separate files, and update imports accordingly
…le module and update references throughout the codebase
…nd IterableActionSource to separate files and update imports accordingly
…pdate imports, and introduce IterableLogLevel enum
… IterableConfig, and create IterableAuthResponse class
…terableAttributionInfo, and IterableCommerceItem; consolidate imports from enums
…ssage: update to named imports for consistency
…e exports, update paths for consistency, and enhance organization
… IterableInboxCustomizations, IterableInboxImpressionRowInfo, and IterableInboxRowViewModel
@lposen lposen changed the title [MOB-9995] reorganize-source-code [MOB-9995] reorganize source code Nov 14, 2024
…com:Iterable/react-native-sdk into 2.0.0-alpha/MOB-9995-reorganize-source-code

* '2.0.0-alpha/MOB-9995-reorganize-source-code' of github.com:Iterable/react-native-sdk:
  Add environment setup instructions to README: include example PATH configurations for rbenv, nvm, Android SDK, Java, and Node
  Add troubleshooting steps for Android SDK location errors in README
  Update Gradle and Android Gradle Plugin versions: upgrade to Gradle 8.9 and Android Gradle Plugin 8.7.2
  Add Ruby version check to README: include instructions for verifying the correct Ruby version is loading
  Fix typo in README: add missing period to installation instruction
  Remove Gemfile.lock and update README with troubleshooting steps for common errors
Copy link

github-actions bot commented Nov 14, 2024

Lines Statements Branches Functions
Coverage: 36%
36.63% (174/475) 11.56% (20/173) 31.84% (50/157)

Copy link
Contributor

@evantk91 evantk91 left a comment

Choose a reason for hiding this comment

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

Looking great! Thoroughly looked through all the imports and ran the example apps.

import IterableInboxEmptyState from '../IterableInboxEmptyState';
import IterableInboxMessageDisplay from '../IterableInboxMessageDisplay';
import IterableInboxMessageList from '../IterableInboxMessageList';
import { useAppStateListener, useDeviceOrientation } from '../hooks';

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice use of the index file. The imports are very organized.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks!

} from './IterableInAppClasses';
import IterableInAppManager from './IterableInAppManager';
import IterableInAppMessage from './IterableInAppMessage';
} from './inApp';
import { IterableLogger } from './IterableLogger';
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice I like the reorganization

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great! I'm glad


static fromDict(dict: any): IterableInboxMetadata {
return new IterableInboxMetadata(dict.title, dict.subtitle, dict.icon);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Breaking up IterableInAppClasses looks good. Checked all the associated classes are covered. Nice reorganization.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks!

src/Iterable.ts Outdated

const RNIterableAPI = NativeModules.RNIterableAPI;
const RNEventEmitter = new NativeEventEmitter(RNIterableAPI);

enum AuthResponseCallback {
enum IterableAuthResponseResult {
SUCCESS,
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice rename.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! Trying to make sure that it isn't confusing for the client when importing things from our code.

@@ -640,7 +592,7 @@ export class Iterable {
}

if (Iterable.savedConfig.authHandler) {
let authResponseCallback: AuthResponseCallback;
let authResponseCallback: IterableAuthResponseResult;
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to rename variable authResponseCallback to authResponseResult to align?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As it is internal, I don't think it is really necessary

handleAuthFailureCalled = 'handleAuthFailureCalled',
}

export default IterableEventName;
Copy link
Contributor

Choose a reason for hiding this comment

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

Same.

import { IterableDataRegion } from './IterableDataRegion';
import { IterablePushPlatform } from './IterablePushPlatform';
import { IterableLogLevel } from './enums';
import { IterableInAppMessage, IterableInAppShowResponse } from './inApp';

// TODO: Add description
type AuthCallBack = () => void;

Copy link
Contributor

Choose a reason for hiding this comment

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

Why was this deleted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's such a simple type that it's not really needed

tsconfig.json Outdated
"inbox/*": ["./src/inbox/*"],
"inbox": ["./src/inbox/index"],
"inApp/*": ["./src/inApp/*"],
"inApp": ["./src/inApp/index"],
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure exactly what this change is for

@@ -0,0 +1,3 @@
export * from './classes';
export * from './enums';
export * from './hooks';
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice reorganization. Didn't know you could have nested index files.

@@ -7,5 +7,3 @@ export enum IterableInAppTriggerType {
/** Do not display the in-app automatically via the SDK */
never = 2,
}

export default IterableInAppTriggerType;
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah I see. Disregard the comments on this. :)

Base automatically changed from 2.0.0-alpha/MOB-10131-remove-duplicate-code to 2.0.0-alpha/master December 3, 2024 21:05
@lposen lposen merged commit d7256fd into 2.0.0-alpha/master Dec 3, 2024
2 of 4 checks passed
@lposen lposen deleted the 2.0.0-alpha/MOB-9995-reorganize-source-code branch December 3, 2024 21:08
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.

2 participants