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
FIX: update duration & interval when input values updated. #26796
Conversation
Previously, we only updated the duration and interval values in the constructor. So whenever the initial values are updated in the form the changes are not reflected in the UI. To fix this issue we're using "get" methods in this PR.
const category = fixturesByUrl["/c/11/show.json"].category; | ||
category.permissions = []; |
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've got a leak! 🪣💧
(gotta wrap the category
in a cloneJSON()
)
app/assets/javascripts/discourse/tests/acceptance/category-edit-settings-test.js
Outdated
Show resolved
Hide resolved
app/assets/javascripts/discourse/tests/acceptance/category-edit-settings-test.js
Outdated
Show resolved
Hide resolved
app/assets/javascripts/discourse/app/components/relative-time-picker.gjs
Outdated
Show resolved
Hide resolved
_intervalFromHours() { | ||
let interval; | ||
if (this.args.durationHours === null) { | ||
this.duration = this.args.durationHours; | ||
this.selectedInterval = "hours"; | ||
interval = "hours"; | ||
} else if (this.args.durationHours >= 8760) { | ||
this.duration = this._roundedDuration(this.args.durationHours / 365 / 24); | ||
this.selectedInterval = "years"; | ||
interval = "years"; | ||
} else if (this.args.durationHours >= 730) { | ||
this.duration = this._roundedDuration(this.args.durationHours / 30 / 24); | ||
this.selectedInterval = "months"; | ||
interval = "months"; | ||
} else if (this.args.durationHours >= 24) { | ||
this.duration = this._roundedDuration(this.args.durationHours / 24); | ||
this.selectedInterval = "days"; | ||
interval = "days"; | ||
} else if (this.args.durationHours < 1) { | ||
this.duration = this._roundedDuration(this.args.durationHours * 60); | ||
this.selectedInterval = "mins"; | ||
interval = "mins"; | ||
} else { | ||
this.duration = this.args.durationHours; | ||
this.selectedInterval = "hours"; | ||
interval = "hours"; | ||
} | ||
return interval; |
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.
Similar here:
if (this.args.durationHours === null) {
return "hours";
} else if (this.args.durationHours >= 8760) {
return "years";
} else if (this.args.durationHours >= 730) {
return "months";
} else if (this.args.durationHours >= 24) {
return "days";
} else if (this.args.durationHours >= 1) {
return "hours";
} else {
return "mins";
}
let duration; | ||
if (this.args.durationMinutes >= 525600) { | ||
this.duration = this._roundedDuration( | ||
duration = this._roundedDuration( | ||
this.args.durationMinutes / 365 / 60 / 24 | ||
); | ||
this.selectedInterval = "years"; | ||
} else if (this.args.durationMinutes >= 43800) { | ||
this.duration = this._roundedDuration( | ||
duration = this._roundedDuration( | ||
this.args.durationMinutes / 30 / 60 / 24 | ||
); | ||
this.selectedInterval = "months"; | ||
} else if (this.args.durationMinutes >= 1440) { | ||
this.duration = this._roundedDuration( | ||
this.args.durationMinutes / 60 / 24 | ||
); | ||
this.selectedInterval = "days"; | ||
duration = this._roundedDuration(this.args.durationMinutes / 60 / 24); | ||
} else if (this.args.durationMinutes >= 60) { | ||
duration = this._roundedDuration(this.args.durationMinutes / 60); | ||
} else { | ||
duration = this.args.durationMinutes; | ||
} | ||
return duration; |
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.
and here, and the method below.
btw, all 4 should be getters instead.
…t-settings-test.js Co-authored-by: Jarek Radosz <jradosz@gmail.com>
…t-settings-test.js Co-authored-by: Jarek Radosz <jradosz@gmail.com>
…picker.gjs Co-authored-by: Jarek Radosz <jradosz@gmail.com>
Previously, we only updated the duration and interval values in the constructor. So whenever the initial values are updated in the form the changes are not reflected in the UI. To fix this issue we're using "get" methods in this PR.