You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by GitBox <gi...@apache.org> on 2020/07/29 07:36:02 UTC
[GitHub] [cloudstack-primate] ravening opened a new pull request #559: Fix global settings name display issue
ravening opened a new pull request #559:
URL: https://github.com/apache/cloudstack-primate/pull/559
Global setting name is displayed twice
After the fix
----------------------------------------------------------------
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] [cloudstack-primate] ravening commented on a change in pull request #559: Fix global settings name display issue
Posted by GitBox <gi...@apache.org>.
ravening commented on a change in pull request #559:
URL: https://github.com/apache/cloudstack-primate/pull/559#discussion_r462193958
##########
File path: src/components/view/ListView.vue
##########
@@ -68,7 +68,7 @@
<os-logo v-if="record.ostypename" :osName="record.ostypename" size="1x" style="margin-right: 5px" />
<console :resource="record" size="small" style="margin-right: 5px" />
- <span v-if="$route.path.startsWith('/globalsetting')">{{ text }}</span>
+ <span v-if="$route.path.startsWith('/globalsetting')"></span>
<span v-if="$route.path.startsWith('/alert')">
Review comment:
@davidjumani can explain more? I didnt understand about the v-else-if part
##########
File path: src/components/view/ListView.vue
##########
@@ -68,7 +68,7 @@
<os-logo v-if="record.ostypename" :osName="record.ostypename" size="1x" style="margin-right: 5px" />
<console :resource="record" size="small" style="margin-right: 5px" />
- <span v-if="$route.path.startsWith('/globalsetting')">{{ text }}</span>
+ <span v-if="$route.path.startsWith('/globalsetting')"></span>
<span v-if="$route.path.startsWith('/alert')">
Review comment:
@davidjumani the line `<span v-if="$route.path.startsWith('/globalsetting')">{{ text }}</span>` is displaying both plain text and the router link as shown in the screenshot above
##########
File path: src/components/view/ListView.vue
##########
@@ -68,7 +68,7 @@
<os-logo v-if="record.ostypename" :osName="record.ostypename" size="1x" style="margin-right: 5px" />
<console :resource="record" size="small" style="margin-right: 5px" />
- <span v-if="$route.path.startsWith('/globalsetting')">{{ text }}</span>
+ <span v-if="$route.path.startsWith('/globalsetting')"></span>
<span v-if="$route.path.startsWith('/alert')">
Review comment:
@davidjumani done
----------------------------------------------------------------
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] [cloudstack-primate] davidjumani commented on pull request #559: Fix global settings name display issue
Posted by GitBox <gi...@apache.org>.
davidjumani commented on pull request #559:
URL: https://github.com/apache/cloudstack-primate/pull/559#issuecomment-665604746
@blueorangutan package
----------------------------------------------------------------
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] [cloudstack-primate] davidjumani commented on a change in pull request #559: Fix global settings name display issue
Posted by GitBox <gi...@apache.org>.
davidjumani commented on a change in pull request #559:
URL: https://github.com/apache/cloudstack-primate/pull/559#discussion_r462217727
##########
File path: src/components/view/ListView.vue
##########
@@ -68,7 +68,7 @@
<os-logo v-if="record.ostypename" :osName="record.ostypename" size="1x" style="margin-right: 5px" />
<console :resource="record" size="small" style="margin-right: 5px" />
- <span v-if="$route.path.startsWith('/globalsetting')">{{ text }}</span>
+ <span v-if="$route.path.startsWith('/globalsetting')"></span>
<span v-if="$route.path.startsWith('/alert')">
Review comment:
can change it to
```
<span v-if="$route.path.startsWith('/globalsetting')">{{ text }}</span>
<span v-else-if="$route.path.startsWith('/alert')">
```
So that the name is plain text and not a router link to a detail view of the global setting which is pretty much just the name
##########
File path: src/components/view/ListView.vue
##########
@@ -68,7 +68,7 @@
<os-logo v-if="record.ostypename" :osName="record.ostypename" size="1x" style="margin-right: 5px" />
<console :resource="record" size="small" style="margin-right: 5px" />
- <span v-if="$route.path.startsWith('/globalsetting')">{{ text }}</span>
+ <span v-if="$route.path.startsWith('/globalsetting')"></span>
<span v-if="$route.path.startsWith('/alert')">
Review comment:
@ravening can change it to
```
<span v-if="$route.path.startsWith('/globalsetting')">{{ text }}</span>
<span v-else-if="$route.path.startsWith('/alert')">
```
So that the name is plain text and not a router link to a detail view of the global setting which is pretty much just the name
----------------------------------------------------------------
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] [cloudstack-primate] rhtyd commented on pull request #559: Fix global settings name display issue
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #559:
URL: https://github.com/apache/cloudstack-primate/pull/559#issuecomment-665564818
@ravening can you address review comments, thanks.
----------------------------------------------------------------
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] [cloudstack-primate] blueorangutan commented on pull request #559: Fix global settings name display issue
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #559:
URL: https://github.com/apache/cloudstack-primate/pull/559#issuecomment-665605399
----------------------------------------------------------------
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] [cloudstack-primate] davidjumani commented on a change in pull request #559: Fix global settings name display issue
Posted by GitBox <gi...@apache.org>.
davidjumani commented on a change in pull request #559:
URL: https://github.com/apache/cloudstack-primate/pull/559#discussion_r461497864
##########
File path: src/components/view/ListView.vue
##########
@@ -68,7 +68,7 @@
<os-logo v-if="record.ostypename" :osName="record.ostypename" size="1x" style="margin-right: 5px" />
<console :resource="record" size="small" style="margin-right: 5px" />
- <span v-if="$route.path.startsWith('/globalsetting')">{{ text }}</span>
+ <span v-if="$route.path.startsWith('/globalsetting')"></span>
<span v-if="$route.path.startsWith('/alert')">
Review comment:
@ravening would be better to make this a v-else-if and leave the prior line as is since we don't need a link and detail view for each global setting
----------------------------------------------------------------
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] [cloudstack-primate] rhtyd merged pull request #559: Fix global settings name display issue
Posted by GitBox <gi...@apache.org>.
rhtyd merged pull request #559:
URL: https://github.com/apache/cloudstack-primate/pull/559
----------------------------------------------------------------
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