-
Notifications
You must be signed in to change notification settings - Fork 45
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
Change name for interne deployment #10302
base: dev
Are you sure you want to change the base?
Conversation
- change logo stdcm when using application internally Signed-off-by: Uriel-Sautron <[email protected]>
…cation internally
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
Thanks for the PR !
In my understanding of the front (which might get a bit rusty over time) I think we should reference directly only the files that are present in the code base (the default logo) and switch if the override file exists on the server and defines the fields on the json.
front/src/utils/hooks/useLogo.ts
Outdated
stdcmLogo: undefined, | ||
stdcmPngLogo: defaultLogo, | ||
}); | ||
const [isIntenalProd, setIsIntenalProd] = useState(false); |
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 we should not name this internalProd, but "customizedDeployment", it seems more accurate.
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.
Done
@@ -15,6 +16,7 @@ import type { SimulationReportSheetProps } from '../../types'; | |||
import { getStopDurationTime } from '../../utils/formatSimulationReportSheet'; | |||
|
|||
const getSecondaryCode = ({ location }: StdcmPathStep) => location!.secondary_code; | |||
const lmrLogoPath = '../../overrides/[email protected]'; |
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 should not reference any override directly in terms of path, they are dynamically defined by the JSON.
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.
Done
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.
Overall LGTM, thanks !
|
||
const lmrLogoPath = `../../overrides/${icons.stdcm.light}.svg`; | ||
const lmrPngLogoPath = `../../overrides/${icons.stdcm.light}@2x.png`; | ||
const horizonFullLogoPath = `../../overrides/${icons.digital_twin.dark}.svg`; |
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.
Any reasons to use ../../overrides vs /overrides ?
close #10281
For internal deployment we need to display the LMR and Horizon logos.
For test:
overrides
at the front rootoverrides.json