-
Notifications
You must be signed in to change notification settings - Fork 25
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
Using license extension URL from the ServiceControl API #2108
Conversation
Co-authored-by: WilliamBZA <WilliamBZA@users.noreply.github.com>
Co-authored-by: John Simons <john.simons@particular.net>
@@ -18,7 +19,7 @@ function displayWarningMessage(licenseStatus: LicenseStatus) { | |||
break; | |||
} | |||
case "ValidWithExpiringTrial": { | |||
const trialExpiring = `<div><strong>Non-production development license expiring</strong><div>Your non-production development license will expire soon. To continue using the Particular Service Platform you'll need to extend your license.</div><a href="http://particular.net/extend-your-trial?p=servicepulse" class="btn btn-warning"><i class="fa fa-external-link-alt"></i> Extend your license</a><a href="${configurationRootLink}" class="btn btn-light">View license details</a></div>`; | |||
const trialExpiring = `<div><strong>Non-production development license expiring</strong><div>Your non-production development license will expire soon. To continue using the Particular Service Platform you'll need to extend your license.</div><a href="${license.license_extension_url}" class="btn btn-warning"><i class="fa fa-external-link-alt"></i> Extend your license</a><a href="${configurationRootLink}" class="btn btn-light">View license details</a></div>`; |
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.
Something feels off here, we shouldn’t be writing html as strings, can this be extracted to a component?
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.
I've crated a dedicated refactoring issue to capture that #2116
@@ -62,7 +62,7 @@ const loading = computed(() => { | |||
</span> | |||
<div class="license-expired-text" v-if="licenseStatus.isPlatformTrialExpired">Your license expired. To continue using the Particular Service Platform you'll need to extend your license.</div> | |||
<div class="license-page-extend-trial" v-if="licenseStatus.isPlatformTrialExpiring && licenseStatus.isPlatformTrialExpired"> | |||
<a class="btn btn-default btn-primary" href="https://particular.net/extend-your-trial?p=servicepulse" target="_blank">Extend your license <i class="fa fa-external-link"></i></a> | |||
<a class="btn btn-default btn-primary" :href="license.license_extension_url" target="_blank">Extend your license <i class="fa fa-external-link"></i></a> |
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.
It may be the right time to create an ExternalLink component
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.
I'm missing a lot of context when in comes to Vue.js. Could you explain the benefit of introducing an ExternalLink component?
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.
At the moment a dev needs to know all the styles, the external icon, the spaces between the icon and text ….
Those are all internal concerns.
With something like
<ExternalLink href=x />
they don’t need to know about those concerns. Also that makes it easier to update the external link look and feel in a single component.
In webapps now a days with ui frameworks most html is abstracted away.
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.
Makes sense. I've captured that in an issue #2117
No description provided.