-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Adding filter-buttons in pod list view (#21) #8444
base: develop
Are you sure you want to change the base?
Conversation
* Filter buttons for Pod view
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 wasn't the one who was requested for review, but since there was no review yet I still left some feedback ;)
@@ -9,12 +9,15 @@ de: | |||
admin: | |||
pods: | |||
actions: "Aktionen" | |||
activePodsFilter: "Verfügbar" |
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.
You need to add localization to the English files, all other languages are managed by webtranslateit and will be overwritten. And only the English gets uploaded to webtranslateit, so if it's not in the English file, it will never be available to be translated into other languages. And if it's missing in english it also means that it breaks the pods page for all other languages (they just show a white page), as there is not even a fallback translation available.
showBlockedPods: function() { | ||
this.podfilter = "blocked"; | ||
this.postRenderTemplate(); | ||
this.$("#show_all_pods").removeClass("active"); | ||
this.$("#show_active_pods").removeClass("active"); | ||
this.$("#show_invalid_pods").removeClass("active"); | ||
}, |
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.
There are no blocked pods (yet).
var versionFailed = $("<div class='alert alert-warning' role='alert' />") | ||
.append(Diaspora.I18n.t("admin.pods.version_failed", {count: gon.versionFailedCount})); | ||
.append(Diaspora.I18n.t("admin.pods.version_failed", {count: gon.versionFailedCount.toLocaleString()})); |
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.
Why is this .toLocaleString()
here, but only for these two? 🤔
this.$("#show_all_pods").addClass("active"); | ||
this.$("#show_active_pods").removeClass("active"); | ||
this.$("#show_invalid_pods").removeClass("active"); |
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 don't know if there is a better solution than manually adding and removing the active
class everywhere? Because that doesn't scale that well if there are are more buttons (as it looks like you plan to add one for blocked pods in the future).
I don't know if there is a possibility to group these buttons somehow so only one can be active at once, no matter how many buttons exist (like radio buttons in forms?). Maybe just remove it from all buttons and just add it back to the one that was clicked?
model: pod | ||
}).render()); | ||
if (self.podfilter === "" || | ||
self.podfilter === "active" && pod.get("status") === "no_errors" || |
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.
Maybe change this one to check the active
field instead? This would then include all pods which are being handled as "active" in the backend (all pods to which is being federated). This also includes pods with warnings (but which are still online and working), and pods which only have errors since less than 14 days (so maybe only a temporary downtime?).
If you want to filter for "no error" then the label on the button text should probably say something like "no error", because "active" means something different.
On Pods with large lists of pods - the admin pod list is mixed with available and invalid pods.
This PR adds a simple filter for 'active' and 'invalid' pods.