-
Notifications
You must be signed in to change notification settings - Fork 64
Support availableLanguages
key
#968
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -404,6 +404,10 @@ export default { | |
required: false, | ||
validator: v => Object.prototype.hasOwnProperty.call(StandardColors, v), | ||
}, | ||
availableLanguages: { | ||
type: Array, | ||
required: false, | ||
}, | ||
availableLocales: { | ||
type: Array, | ||
required: false, | ||
|
@@ -624,6 +628,7 @@ export default { | |
conformance, | ||
hasNoExpandedDocumentation, | ||
modules, | ||
availableLanguages, | ||
availableLocales, | ||
platforms, | ||
required: isRequirement = false, | ||
|
@@ -666,6 +671,7 @@ export default { | |
downloadNotAvailableSummary, | ||
diffAvailability, | ||
hasNoExpandedDocumentation, | ||
availableLanguages, | ||
availableLocales, | ||
hierarchy, | ||
role, | ||
|
@@ -717,7 +723,7 @@ export default { | |
}); | ||
} | ||
|
||
AppStore.setAvailableLocales(this.availableLocales || []); | ||
AppStore.setAvailableLocales(this.availableLanguages ?? this.availableLocales); | ||
this.store.reset(); | ||
this.store.setReferences(this.references); | ||
}, | ||
|
@@ -729,8 +735,11 @@ export default { | |
references(references) { | ||
this.store.setReferences(references); | ||
}, | ||
availableLanguages(availableLanguages) { | ||
AppStore.setAvailableLocales(availableLanguages); | ||
}, | ||
availableLocales(availableLocales) { | ||
AppStore.setAvailableLocales(availableLocales); | ||
AppStore.setAvailableLocales(this.availableLanguages ?? availableLocales); | ||
Comment on lines
+738
to
+742
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar feedback: We could make a computed property like resolvedAvailableLanguages({ availableLanguages, availableLocales }) {
return availableLanguages ?? availableLocales;
} and then have a watcher like watch: {
resolvedAvailableLanguages(newValue) {
AppStore.setAvailableLocales(newValue);
},
}, on AppStore.setAvailableLocales(this.resolvedAvailableLanguages); |
||
}, | ||
}, | ||
}; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -135,7 +135,9 @@ export default { | |
}, | ||
}, | ||
created() { | ||
AppStore.setAvailableLocales(this.metadata.availableLocales); | ||
AppStore.setAvailableLocales( | ||
this.metadata.availableLanguages ?? this.metadata.availableLocales, | ||
); | ||
this.store.reset(); | ||
this.store.setReferences(this.references); | ||
}, | ||
|
@@ -145,7 +147,10 @@ export default { | |
this.store.setReferences(references); | ||
}, | ||
'metadata.availableLocales': function availableLocalesWatcher(availableLocales) { | ||
AppStore.setAvailableLocales(availableLocales); | ||
AppStore.setAvailableLocales(this.metadata?.availableLanguages ?? availableLocales); | ||
}, | ||
'metadata.availableLanguages': function availableLanguagesWatcher(availableLanguages) { | ||
AppStore.setAvailableLocales(availableLanguages); | ||
Comment on lines
147
to
+153
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same feedback about the computed property as before |
||
}, | ||
}, | ||
mounted() { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -101,7 +101,9 @@ export default { | |
}; | ||
}, | ||
created() { | ||
AppStore.setAvailableLocales(this.metadata.availableLocales); | ||
AppStore.setAvailableLocales( | ||
this.metadata.availableLanguages ?? this.metadata.availableLocales, | ||
); | ||
this.store.reset(); | ||
this.store.setReferences(this.references); | ||
}, | ||
|
@@ -111,7 +113,10 @@ export default { | |
this.store.setReferences(references); | ||
}, | ||
'metadata.availableLocales': function availableLocalesWatcher(availableLocales) { | ||
AppStore.setAvailableLocales(availableLocales); | ||
AppStore.setAvailableLocales(this.metadata?.availableLanguages ?? availableLocales); | ||
}, | ||
'metadata.availableLanguages': function availableLanguagesWatcher(availableLanguages) { | ||
AppStore.setAvailableLocales(availableLanguages); | ||
Comment on lines
113
to
+119
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same feedback about the computed property as before |
||
}, | ||
}, | ||
}; | ||
|
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.
We could refactor this code by adding a computed property:
and then just watch that computed property like:
instead of watching
'metadata.availableLocales'
and'metadata.availableLanguages'
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.
I agree that the existing logic could be refactored and improved to be better and more consistent, but I was mostly focused on making the minimal changes to minimize the surface area of changes overall.
Thanks for the suggestion! It might be good to make those changes afterwards.