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

fixes #911: add warning to encryption popup #838

Merged
merged 2 commits into from
Aug 9, 2023

Conversation

sadiqkhoja
Copy link
Contributor

@sadiqkhoja sadiqkhoja commented Aug 2, 2023

Related to getodk/central-backend#911

Show warning that "Draft Submissions from all Forms will be deleted."

I took the liberty to remove "In future" texts as well, I can add them back if everyone want that.

image

Before submitting this PR, please make sure you have:

  • run npm run test and npm run lint and confirmed all checks still pass OR confirm CircleCI build passes
  • verified that any code or assets from external sources are properly credited in comments or that everything is internally sourced

Comment on lines 65 to 70
<p>{{ $t('steps[0].introduction[1][1][1]') }}</p>
</div>
<div class="info-item">
<span class="icon-close"></span>
<p>{{ $t('steps[0].introduction[1][2][0]') }}</p>
<p>{{ $t('steps[0].introduction[1][2][1]') }}</p>
Copy link
Member

Choose a reason for hiding this comment

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

I think it's reasonable to remove this explanatory text. There are more items in the modal than there used to be, so I think it's a good idea to remove non-essential text. CC @lognaturel for a second opinion.

Copy link
Member

Choose a reason for hiding this comment

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

If we do remove this explanatory text, we should also remove the i18n messages below.

Copy link
Member

Choose a reason for hiding this comment

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

I’m pro removing all “in future” text. I think it’s more confusing than helpful.

Copy link
Member

Choose a reason for hiding this comment

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

"Existing Submissions will remain unencrypted." seems like a useful caveat to me, but I don't think we need "Encryption cannot be turned off once enabled."

src/components/project/enable-encryption.vue Outdated Show resolved Hide resolved
src/components/project/enable-encryption.vue Outdated Show resolved Hide resolved
src/components/project/enable-encryption.vue Outdated Show resolved Hide resolved
@sadiqkhoja
Copy link
Contributor Author

image

What do you think about this?

I have removed "In addition, the following are true in this version of ODK Central"

I don't think we need "Encryption cannot be turned off once enabled."

I think this is important thing to mention

@matthew-white
Copy link
Member

matthew-white commented Aug 9, 2023

Looks great! 💯

Even though there will be just one list, could you try to preserve the structure/order of the messages in the i18n JSON? Otherwise translations will be removed in Transifex. Or you could use a @transifexKey comment to move a message while preserving its translations in Transifex.

@sadiqkhoja sadiqkhoja merged commit 5ccb2ed into getodk:master Aug 9, 2023
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.

3 participants