Skip to content

Commit

Permalink
Fix missing options processing (wix#7370)
Browse files Browse the repository at this point in the history
We missed `mergeOptions` processing on the following commands: `pop`, `popTo`, `popToRoot`, `dismissModal` and `dismissAllModals`. That resulted in a bug where using the old animations syntax didn't work.
  • Loading branch information
yogevbd authored Nov 25, 2021
1 parent e0da767 commit 623c239
Show file tree
Hide file tree
Showing 5 changed files with 134 additions and 61 deletions.
68 changes: 67 additions & 1 deletion lib/src/commands/Commands.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { IWrappedComponent } from 'react-native-navigation/components/ComponentW
describe('Commands', () => {
let uut: Commands;
let mockedNativeCommandsSender: NativeCommandsSender;
let mockedOptionsProcessor: OptionsProcessor;
let mockedStore: Store;
let commandsObserver: CommandsObserver;
let mockedUniqueIdProvider: UniqueIdProvider;
Expand All @@ -36,7 +37,7 @@ describe('Commands', () => {
commandsObserver = new CommandsObserver(uniqueIdProvider);
const layoutProcessorsStore = new LayoutProcessorsStore();

const mockedOptionsProcessor = mock(OptionsProcessor);
mockedOptionsProcessor = mock(OptionsProcessor);
const optionsProcessor = instance(mockedOptionsProcessor) as OptionsProcessor;

layoutProcessor = new LayoutProcessor(layoutProcessorsStore);
Expand Down Expand Up @@ -317,6 +318,19 @@ describe('Commands', () => {
const result = await uut.dismissModal('myUniqueId');
expect(result).toEqual('the id');
});

it('processes mergeOptions', async () => {
const options = {
animations: {
dismissModal: {
enabled: false,
},
},
};

uut.dismissModal('myUniqueId', options);
verify(mockedOptionsProcessor.processOptions(CommandName.DismissModal, options)).called();
});
});

describe('dismissAllModals', () => {
Expand All @@ -334,6 +348,19 @@ describe('Commands', () => {
const result = await uut.dismissAllModals();
expect(result).toEqual('the id');
});

it('processes mergeOptions', async () => {
const options: Options = {
animations: {
dismissModal: {
enabled: false,
},
},
};

uut.dismissAllModals(options);
verify(mockedOptionsProcessor.processOptions(CommandName.DismissAllModals, options)).called();
});
});

describe('push', () => {
Expand Down Expand Up @@ -405,6 +432,19 @@ describe('Commands', () => {
const result = await uut.pop('theComponentId', {});
expect(result).toEqual('theComponentId');
});

it('processes mergeOptions', async () => {
const options: Options = {
animations: {
pop: {
enabled: false,
},
},
};

uut.pop('theComponentId', options);
verify(mockedOptionsProcessor.processOptions(CommandName.Pop, options)).called();
});
});

describe('popTo', () => {
Expand All @@ -422,6 +462,19 @@ describe('Commands', () => {
const result = await uut.popTo('theComponentId');
expect(result).toEqual('theComponentId');
});

it('processes mergeOptions', async () => {
const options: Options = {
animations: {
pop: {
enabled: false,
},
},
};

uut.popTo('theComponentId', options);
verify(mockedOptionsProcessor.processOptions(CommandName.PopTo, options)).called();
});
});

describe('popToRoot', () => {
Expand All @@ -439,6 +492,19 @@ describe('Commands', () => {
const result = await uut.popToRoot('theComponentId');
expect(result).toEqual('theComponentId');
});

it('processes mergeOptions', async () => {
const options: Options = {
animations: {
pop: {
enabled: false,
},
},
};

uut.popToRoot('theComponentId', options);
verify(mockedOptionsProcessor.processOptions(CommandName.PopToRoot, options)).called();
});
});

describe('setStackRoot', () => {
Expand Down
7 changes: 6 additions & 1 deletion lib/src/commands/Commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ export class Commands {

public mergeOptions(componentId: string, options: Options) {
const input = cloneDeep(options);
this.optionsProcessor.processOptions(input, CommandName.MergeOptions);
this.optionsProcessor.processOptions(CommandName.MergeOptions, input);

const component = this.store.getComponentInstance(componentId);
if (component && !component.isMounted)
Expand Down Expand Up @@ -106,6 +106,7 @@ export class Commands {

public dismissModal(componentId: string, mergeOptions?: Options) {
const commandId = this.uniqueIdProvider.generate(CommandName.DismissModal);
this.optionsProcessor.processOptions(CommandName.DismissModal, mergeOptions);
const result = this.nativeCommandsSender.dismissModal(commandId, componentId, mergeOptions);
this.commandsObserver.notify(CommandName.DismissModal, {
commandId,
Expand All @@ -117,6 +118,7 @@ export class Commands {

public dismissAllModals(mergeOptions?: Options) {
const commandId = this.uniqueIdProvider.generate(CommandName.DismissAllModals);
this.optionsProcessor.processOptions(CommandName.DismissAllModals, mergeOptions);
const result = this.nativeCommandsSender.dismissAllModals(commandId, mergeOptions);
this.commandsObserver.notify(CommandName.DismissAllModals, { commandId, mergeOptions });
return result;
Expand All @@ -138,20 +140,23 @@ export class Commands {

public pop(componentId: string, mergeOptions?: Options) {
const commandId = this.uniqueIdProvider.generate(CommandName.Pop);
this.optionsProcessor.processOptions(CommandName.Pop, mergeOptions);
const result = this.nativeCommandsSender.pop(commandId, componentId, mergeOptions);
this.commandsObserver.notify(CommandName.Pop, { commandId, componentId, mergeOptions });
return result;
}

public popTo(componentId: string, mergeOptions?: Options) {
const commandId = this.uniqueIdProvider.generate(CommandName.PopTo);
this.optionsProcessor.processOptions(CommandName.PopTo, mergeOptions);
const result = this.nativeCommandsSender.popTo(commandId, componentId, mergeOptions);
this.commandsObserver.notify(CommandName.PopTo, { commandId, componentId, mergeOptions });
return result;
}

public popToRoot(componentId: string, mergeOptions?: Options) {
const commandId = this.uniqueIdProvider.generate(CommandName.PopToRoot);
this.optionsProcessor.processOptions(CommandName.PopToRoot, mergeOptions);
const result = this.nativeCommandsSender.popToRoot(commandId, componentId, mergeOptions);
this.commandsObserver.notify(CommandName.PopToRoot, { commandId, componentId, mergeOptions });
return result;
Expand Down
2 changes: 1 addition & 1 deletion lib/src/commands/LayoutTreeCrawler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export class LayoutTreeCrawler {
if (node.type === LayoutType.Component) {
this.handleComponent(node);
}
this.optionsProcessor.processOptions(node.data.options, commandName);
this.optionsProcessor.processOptions(commandName, node.data.options);
node.children.forEach((value: LayoutNode) => this.crawl(value, commandName));
}

Expand Down
Loading

0 comments on commit 623c239

Please sign in to comment.