You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by GitBox <gi...@apache.org> on 2021/04/23 04:01:12 UTC
[GitHub] [skywalking-rocketbot-ui] horber opened a new pull request #481: Support to configure the maximum number of displayed items
horber opened a new pull request #481:
URL: https://github.com/apache/skywalking-rocketbot-ui/pull/481
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-rocketbot-ui] horber commented on a change in pull request #481: Support to configure the maximum number of displayed items
Posted by GitBox <gi...@apache.org>.
horber commented on a change in pull request #481:
URL: https://github.com/apache/skywalking-rocketbot-ui/pull/481#discussion_r620203517
##########
File path: src/views/components/dashboard/charts/chart-edit.vue
##########
@@ -237,6 +237,16 @@ limitations under the License. -->
@change="setItemConfig({ type: 'height', value: $event.target.value })"
/>
</div>
+ <div class="flex-h mb-5" v-show="!isChartType">
+ <div class="title grey sm">{{ $t('maxItemNum') }}:</div>
+ <input
+ type="number"
+ min="1"
+ class="rk-chart-edit-input long"
+ :value="itemConfig.maxItemNum"
Review comment:
I feel that all the default data is configured on the back end through the 'queryGetAllTemplates' API, There may be a problem with setting the default value at the front end.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-rocketbot-ui] horber commented on a change in pull request #481: Support to configure the maximum number of displayed items
Posted by GitBox <gi...@apache.org>.
horber commented on a change in pull request #481:
URL: https://github.com/apache/skywalking-rocketbot-ui/pull/481#discussion_r621971958
##########
File path: src/views/components/dashboard/charts/chart-edit.vue
##########
@@ -237,6 +237,16 @@ limitations under the License. -->
@change="setItemConfig({ type: 'height', value: $event.target.value })"
/>
</div>
+ <div class="flex-h mb-5" v-show="!isChartType">
+ <div class="title grey sm">{{ $t('maxItemNum') }}:</div>
+ <input
+ type="number"
+ min="1"
+ class="rk-chart-edit-input long"
+ :value="itemConfig.maxItemNum"
Review comment:
ok, It makes sense
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-rocketbot-ui] Fine0830 commented on a change in pull request #481: Support to configure the maximum number of displayed items
Posted by GitBox <gi...@apache.org>.
Fine0830 commented on a change in pull request #481:
URL: https://github.com/apache/skywalking-rocketbot-ui/pull/481#discussion_r622142888
##########
File path: src/views/components/dashboard/charts/chart-edit.vue
##########
@@ -347,14 +350,24 @@ limitations under the License. -->
private isReadSingleValue = false;
private created() {
- this.itemConfig = this.item;
+ this.setDefaultValue((this.itemConfig = this.item));
this.initConfig();
if (!this.itemConfig.independentSelector || this.pageTypes.includes(this.type)) {
return;
}
this.setItemServices();
}
+ private setDefaultValue(itemConfig: any) {
+ const keys: string[] = Object.keys(itemConfig);
+ let key: string = '';
+ for (key in this.itemConfigDefault) {
Review comment:
The approach has some downside. https://stackoverflow.com/questions/500504/why-is-using-for-in-for-array-iteration-a-bad-idea
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-rocketbot-ui] horber closed pull request #481: Support to configure the maximum number of displayed items
Posted by GitBox <gi...@apache.org>.
horber closed pull request #481:
URL: https://github.com/apache/skywalking-rocketbot-ui/pull/481
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-rocketbot-ui] wu-sheng commented on pull request #481: Support to configure the maximum number of displayed items
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #481:
URL: https://github.com/apache/skywalking-rocketbot-ui/pull/481#issuecomment-826048418
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-rocketbot-ui] wu-sheng commented on pull request #481: Support to configure the maximum number of displayed items
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #481:
URL: https://github.com/apache/skywalking-rocketbot-ui/pull/481#issuecomment-826048418
Feature makes sense to me. @Fine0830 Your decision on codes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-rocketbot-ui] horber commented on pull request #481: Support to configure the maximum number of displayed items
Posted by GitBox <gi...@apache.org>.
horber commented on pull request #481:
URL: https://github.com/apache/skywalking-rocketbot-ui/pull/481#issuecomment-825477912
> Make sense to me. BTW, you'd better mention issue first. After discussion, you pull a request and this avoids multiple modifications.
👌
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-rocketbot-ui] Fine0830 edited a comment on pull request #481: Support to configure the maximum number of displayed items
Posted by GitBox <gi...@apache.org>.
Fine0830 edited a comment on pull request #481:
URL: https://github.com/apache/skywalking-rocketbot-ui/pull/481#issuecomment-825374457
Make sense to me. BTW, you'd better mention issue first. After discussion, you pull a request and this avoids multiple modifications.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-rocketbot-ui] Fine0830 commented on pull request #481: Support to configure the maximum number of displayed items
Posted by GitBox <gi...@apache.org>.
Fine0830 commented on pull request #481:
URL: https://github.com/apache/skywalking-rocketbot-ui/pull/481#issuecomment-828289485
Please resolve conflicts.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-rocketbot-ui] Fine0830 merged pull request #481: Support to configure the maximum number of displayed items
Posted by GitBox <gi...@apache.org>.
Fine0830 merged pull request #481:
URL: https://github.com/apache/skywalking-rocketbot-ui/pull/481
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-rocketbot-ui] Fine0830 commented on a change in pull request #481: Support to configure the maximum number of displayed items
Posted by GitBox <gi...@apache.org>.
Fine0830 commented on a change in pull request #481:
URL: https://github.com/apache/skywalking-rocketbot-ui/pull/481#discussion_r619662875
##########
File path: src/views/components/dashboard/charts/chart-edit.vue
##########
@@ -237,6 +237,16 @@ limitations under the License. -->
@change="setItemConfig({ type: 'height', value: $event.target.value })"
/>
</div>
+ <div class="flex-h mb-5" v-show="!isChartType">
+ <div class="title grey sm">{{ $t('maxItemNum') }}:</div>
+ <input
+ type="number"
+ min="1"
+ class="rk-chart-edit-input long"
+ :value="itemConfig.maxItemNum"
Review comment:
Please set the default value of `maxItemNum` to `10`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-rocketbot-ui] horber commented on a change in pull request #481: Support to configure the maximum number of displayed items
Posted by GitBox <gi...@apache.org>.
horber commented on a change in pull request #481:
URL: https://github.com/apache/skywalking-rocketbot-ui/pull/481#discussion_r622773193
##########
File path: src/views/components/dashboard/charts/chart-edit.vue
##########
@@ -347,14 +350,24 @@ limitations under the License. -->
private isReadSingleValue = false;
private created() {
- this.itemConfig = this.item;
+ this.setDefaultValue((this.itemConfig = this.item));
this.initConfig();
if (!this.itemConfig.independentSelector || this.pageTypes.includes(this.type)) {
return;
}
this.setItemServices();
}
+ private setDefaultValue(itemConfig: any) {
+ const keys: string[] = Object.keys(itemConfig);
+ let key: string = '';
+ for (key in this.itemConfigDefault) {
Review comment:
got it, already update. Is that ok?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-rocketbot-ui] Fine0830 commented on a change in pull request #481: Support to configure the maximum number of displayed items
Posted by GitBox <gi...@apache.org>.
Fine0830 commented on a change in pull request #481:
URL: https://github.com/apache/skywalking-rocketbot-ui/pull/481#discussion_r621969829
##########
File path: src/views/components/dashboard/charts/chart-edit.vue
##########
@@ -347,14 +350,24 @@ limitations under the License. -->
private isReadSingleValue = false;
private created() {
- this.itemConfig = this.item;
+ this.setDefaultValue((this.itemConfig = this.item));
this.initConfig();
if (!this.itemConfig.independentSelector || this.pageTypes.includes(this.type)) {
return;
}
this.setItemServices();
}
+ private setDefaultValue(itemConfig: any) {
+ const keys: string[] = Object.keys(itemConfig);
+ let key: string = '';
+ for (key in this.itemConfigDefault) {
Review comment:
I think `for...of` is better than `for ..in`. Could you update this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-rocketbot-ui] Fine0830 commented on a change in pull request #481: Support to configure the maximum number of displayed items
Posted by GitBox <gi...@apache.org>.
Fine0830 commented on a change in pull request #481:
URL: https://github.com/apache/skywalking-rocketbot-ui/pull/481#discussion_r619662875
##########
File path: src/views/components/dashboard/charts/chart-edit.vue
##########
@@ -237,6 +237,16 @@ limitations under the License. -->
@change="setItemConfig({ type: 'height', value: $event.target.value })"
/>
</div>
+ <div class="flex-h mb-5" v-show="!isChartType">
+ <div class="title grey sm">{{ $t('maxItemNum') }}:</div>
+ <input
+ type="number"
+ min="1"
+ class="rk-chart-edit-input long"
+ :value="itemConfig.maxItemNum"
Review comment:
Please set the default value of `maxItemNum` to `10`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-rocketbot-ui] wu-sheng commented on pull request #481: Support to configure the maximum number of displayed items
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #481:
URL: https://github.com/apache/skywalking-rocketbot-ui/pull/481#issuecomment-826048499
And, we should not update all existing templates, please make sure the default value is there and as same as the previous version.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-rocketbot-ui] horber commented on a change in pull request #481: Support to configure the maximum number of displayed items
Posted by GitBox <gi...@apache.org>.
horber commented on a change in pull request #481:
URL: https://github.com/apache/skywalking-rocketbot-ui/pull/481#discussion_r621976741
##########
File path: src/views/components/dashboard/charts/chart-edit.vue
##########
@@ -347,14 +350,24 @@ limitations under the License. -->
private isReadSingleValue = false;
private created() {
- this.itemConfig = this.item;
+ this.setDefaultValue((this.itemConfig = this.item));
this.initConfig();
if (!this.itemConfig.independentSelector || this.pageTypes.includes(this.type)) {
return;
}
this.setItemServices();
}
+ private setDefaultValue(itemConfig: any) {
+ const keys: string[] = Object.keys(itemConfig);
+ let key: string = '';
+ for (key in this.itemConfigDefault) {
Review comment:
why?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-rocketbot-ui] horber commented on pull request #481: Support to configure the maximum number of displayed items
Posted by GitBox <gi...@apache.org>.
horber commented on pull request #481:
URL: https://github.com/apache/skywalking-rocketbot-ui/pull/481#issuecomment-825468285
> Make sense to me. BTW, you'd better mention issue first. After discussion, you pull a request and this avoids multiple modifications.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-rocketbot-ui] horber commented on pull request #481: Support to configure the maximum number of displayed items
Posted by GitBox <gi...@apache.org>.
horber commented on pull request #481:
URL: https://github.com/apache/skywalking-rocketbot-ui/pull/481#issuecomment-825477169
Sorry, I just accidentally turned off this PR
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-rocketbot-ui] horber removed a comment on pull request #481: Support to configure the maximum number of displayed items
Posted by GitBox <gi...@apache.org>.
horber removed a comment on pull request #481:
URL: https://github.com/apache/skywalking-rocketbot-ui/pull/481#issuecomment-825468285
> Make sense to me. BTW, you'd better mention issue first. After discussion, you pull a request and this avoids multiple modifications.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-rocketbot-ui] Fine0830 commented on pull request #481: Support to configure the maximum number of displayed items
Posted by GitBox <gi...@apache.org>.
Fine0830 commented on pull request #481:
URL: https://github.com/apache/skywalking-rocketbot-ui/pull/481#issuecomment-825374457
Make scene to me. BTW, you'd better mention issue first. After discussion, you pull a request and this avoids multiple modifications.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-rocketbot-ui] wu-sheng commented on a change in pull request #481: Support to configure the maximum number of displayed items
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #481:
URL: https://github.com/apache/skywalking-rocketbot-ui/pull/481#discussion_r620208298
##########
File path: src/views/components/dashboard/charts/chart-edit.vue
##########
@@ -237,6 +237,16 @@ limitations under the License. -->
@change="setItemConfig({ type: 'height', value: $event.target.value })"
/>
</div>
+ <div class="flex-h mb-5" v-show="!isChartType">
+ <div class="title grey sm">{{ $t('maxItemNum') }}:</div>
+ <input
+ type="number"
+ min="1"
+ class="rk-chart-edit-input long"
+ :value="itemConfig.maxItemNum"
Review comment:
The default works when this config missing. Which are really missing for all configuration.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-rocketbot-ui] wu-sheng commented on pull request #481: Support to configure the maximum number of displayed items
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #481:
URL: https://github.com/apache/skywalking-rocketbot-ui/pull/481#issuecomment-828982644
@Fine0830 Please take a look when you have time.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org