Skip to content

Switch resource_preview to resource_view #15902

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

Merged
merged 3 commits into from
Jan 24, 2022

Conversation

muzzwood
Copy link
Contributor

What does it do?

Changes the lexicon key from resource_preview to resource_view, and the English words Preview to View.
This is used in the context menu of the resource tree.

Screenshot from 2021-11-12 17-28-23

Why is it needed?

Adds uniformity to action names. The button View on the resource action bar does the same thing but they're named differently.

Screenshot 2021-11-12 at 18-04-16 Editing Home MODX Revolution

The term preview may also make a user think they can see changes without saving.

How to test

Right click on a resource in the tree, or look in recent resources at ?a=security/profile

Related issue(s)/PR(s)

I remember this being mentioned before but if there's an issue I've missed it.

@cla-bot cla-bot bot added the cla-signed CLA confirmed for contributors to this PR. label Nov 12, 2021
@Ibochkarev Ibochkarev added the pr/review-needed Pull request requires review and testing. label Nov 12, 2021
@JoshuaLuckers
Copy link
Contributor

I believe the resource_preview lexicon should be deleted in the language files and not renamed (except for en).
@opengeek Am I right?

@Ibochkarev Ibochkarev added pr/ready-for-merging Pull request reviewed and tested and ready for merging. and removed pr/review-needed Pull request requires review and testing. labels Nov 22, 2021
Copy link
Member

@opengeek opengeek left a comment

Choose a reason for hiding this comment

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

PRs should never modify lexicon files other than the ones in en.

@opengeek opengeek removed the pr/ready-for-merging Pull request reviewed and tested and ready for merging. label Nov 23, 2021
@opengeek
Copy link
Member

I believe the resource_preview lexicon should be deleted in the language files and not renamed (except for en). @opengeek Am I right?

I don't think the language files should be touched at all. Just modify the en/ source.

@Ruslan-Aleev
Copy link
Collaborator

Ruslan-Aleev commented Nov 23, 2021

I don't think the language files should be touched at all. Just modify the en/ source.

Why should we keep unused lexicons? By the way, there are so maaany of them, in fact.

@opengeek
Copy link
Member

I don't think the language files should be touched at all. Just modify the en/ source.

Why should we keep unused lexicons? By the way, there are so maaany of them, in fact.

No one is suggesting keeping unused lexicons. CrowdIn will modify them based on changes to the source (i.e. the en lexicon is the source).

@muzzwood
Copy link
Contributor Author

muzzwood commented Dec 1, 2021

Sorry for the delay, I'll get this updated tonight/tomorrow. Thanks!

@muzzwood muzzwood requested a review from opengeek January 9, 2022 06:12
@Mark-H Mark-H added this to the v3.0.0-rc2 milestone Jan 19, 2022
@Mark-H Mark-H added the pr/ready-for-merging Pull request reviewed and tested and ready for merging. label Jan 19, 2022
@Mark-H
Copy link
Collaborator

Mark-H commented Jan 19, 2022

Setting this to rc2 so there's time for translations to catch up before the release.

@opengeek opengeek merged commit 37a01c0 into modxcms:3.x Jan 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed CLA confirmed for contributors to this PR. pr/ready-for-merging Pull request reviewed and tested and ready for merging.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants