RCLOUD-1270: Hide version from footer for rba #9129
Conversation
ocerda
force-pushed
the
RCLOUD-1270-hide-version-from-footer
branch
2 times, most recently
from
4ed19f0
to
e7f825a
Compare
| :icon="version.icon" | ||
| :color="version.color" | ||
| /> | ||
| <div v-if="!hideVersion"> |
There was a problem hiding this comment.
Just out of curiosity. Why increase the depth of the DOM, when you could change it with a
| <div v-if="!hideVersion"> | |
| <template v-if="!hideVersion"> |
| /> | ||
| </template> | ||
|
|
||
| <script lang="ts"> | ||
| import { RootStore } from "../../../stores/RootStore"; | ||
| import { defineComponent, inject } from "vue"; | ||
| import InfoDisplay from "./RundeckInfo.vue"; | ||
| import {getFeatureEnabled} from "@/library/services/featureConfig"; |
There was a problem hiding this comment.
Try to use relative imports
|
|
||
| export async function getFeatureEnabled(featureName: string) { | ||
| const resp = await api.get(`/feature/${featureName}`); | ||
| return !!resp.data.enabled; |
There was a problem hiding this comment.
What happens if resp is null?
| if (val !== null) { | ||
| this.hideVersion = val; | ||
| } else { | ||
| this.hideVersion = false; | ||
| } |
There was a problem hiding this comment.
This could be done in one line
| if (val !== null) { | |
| this.hideVersion = val; | |
| } else { | |
| this.hideVersion = false; | |
| } | |
| this.hideVersion = (val !== null) ? val : false; |
|
Just a heads-up, there are unit tests for the RundeckInfo component (RundeckInfo.spec.ts), could you please add a test for the new case? |
| const rundeckVersionComponent = wrapper.findComponent({ | ||
| name: "RundeckVersion", | ||
| }); | ||
| const versionDisplayComponent = wrapper.findComponent({ | ||
| name: "VersionDisplay", | ||
| }); |
There was a problem hiding this comment.
The idea of testing is to check that what is shown to the user is tested, so we need to change this to the right behavior, that some divs inside those components are displayed, so you should change that find elements and then check its existance
| const rundeckVersionComponent = wrapper.findComponent({ | |
| name: "RundeckVersion", | |
| }); | |
| const versionDisplayComponent = wrapper.findComponent({ | |
| name: "VersionDisplay", | |
| }); | |
| const rundeckVersionComponent = wrapper.find('div.rundeck-version-display'); | |
| const versionDisplayComponent = wrapper.find('span.rundeck-version-icon'); |
Please check if that works if you want to change it, I'm a bit rusty with those kind fo tests 😅
ocerda
force-pushed
the
RCLOUD-1270-hide-version-from-footer
branch
from
326b620
to
06b81ba
Compare
Is this a bugfix, or an enhancement? Please describe.
Change of behavior for rba
Describe the solution you've implemented
Adding the option to hide the version number from the footer
Describe alternatives you've considered
Additional context
https://pagerduty.atlassian.net/browse/RCLOUD-1270