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

Desktop: Add a banner asking users to install templates plugin #5164

Merged
merged 4 commits into from
Jul 9, 2021
Merged

Desktop: Add a banner asking users to install templates plugin #5164

merged 4 commits into from
Jul 9, 2021

Conversation

nishantwrp
Copy link
Contributor

Add a banner asking users to install templates plugin. It checks for two things

  • If the user has at least one template.
  • User has not already installed the templates plugin.

Comment on lines 697 to 714
const templatesDir = `${Setting.value('profileDir')}/templates`;
if (await shim.fsDriver().exists(templatesDir)) {
try {
const files = await shim.fsDriver().readDirStats(templatesDir);
for (const file of files) {
if (file.path.endsWith('.md')) {
// There is atleast one template.
this.store().dispatch({
type: 'CONTAINS_LEGACY_TEMPLATES',
});
break;
}
}
} catch (error) {
reg.logger().error(`Failed to read templates directory: ${error}`);
}
}

Copy link
Owner

Choose a reason for hiding this comment

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

Could you move this code to a class method and call it from here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -551,6 +552,16 @@ class MainScreenComponent extends React.Component<Props, State> {
});
};

const onPluginScreen = () => {
Copy link
Owner

Choose a reason for hiding this comment

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

onViewPluginScreen

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -639,6 +650,12 @@ class MainScreenComponent extends React.Component<Props, State> {
_('Set the password'),
onViewEncryptionConfigScreen
);
} else if (this.props.showInstallTemplatesPlugin) {
msg = this.renderNotificationMessage(
_('Templates feature is now moved to a plugin called "Templates".'),
Copy link
Owner

Choose a reason for hiding this comment

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

The template feature has been moved to a plugin called "Templates"

Please don't translate as it's temporary (remove the _())

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

} else if (this.props.showInstallTemplatesPlugin) {
msg = this.renderNotificationMessage(
_('Templates feature is now moved to a plugin called "Templates".'),
_('Install Plugin'),
Copy link
Owner

Choose a reason for hiding this comment

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

"Install plugin". And remove _()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -78,6 +78,7 @@ export interface State {
hasDisabledSyncItems: boolean;
hasDisabledEncryptionItems: boolean;
customCss: string;
has_legacy_templates: boolean;
Copy link
Owner

Choose a reason for hiding this comment

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

Come on - look at the code around a bit. There's like a hundred variables in camel case here, and you set yours with underscores...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@nishantwrp
Copy link
Contributor Author

I've addressed all the review comments. PTAL!

@nishantwrp nishantwrp requested a review from laurent22 July 8, 2021 21:19
@laurent22
Copy link
Owner

Looks good now, thanks for the update.

@laurent22 laurent22 merged commit 13c6206 into laurent22:dev_no_template Jul 9, 2021
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