-
Notifications
You must be signed in to change notification settings - Fork 13.1k
[FIX] Asset settings description not showing on admin #26755
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
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #26755 +/- ##
===========================================
+ Coverage 40.61% 40.81% +0.19%
===========================================
Files 790 790
Lines 17903 17903
Branches 1927 1927
===========================================
+ Hits 7272 7307 +35
+ Misses 10342 10300 -42
- Partials 289 296 +7
Flags with carried forward coverage won't be shown. Click here to find out more. |
apps/meteor/client/views/admin/settings/inputs/AssetSettingInput.tsx
Outdated
Show resolved
Hide resolved
apps/meteor/client/views/admin/settings/inputs/AssetSettingInput.tsx
Outdated
Show resolved
Hide resolved
ggazzo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you instead of using data-qa-something make use of role=button[name=something]? I think it's much better and it guarantees that we have semantic access to the buttons
f78a5a6 to
55719c2
Compare
Proposed changes (including videos or screenshots)
Issue(s)
Steps to test or reproduce
Go to Administration > Settings > Assets is not showing the setting description
Further comments
What:
Remove validation to check if the translation key exists to fulfill the settings label.
Why:
After a typescript rewrite was added validation to check if translation key exists, but some settings label doesn't have a translation key, in this case, the label value was
undefined.