-
Notifications
You must be signed in to change notification settings - Fork 8
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
Choose landing page from templates #48
Choose landing page from templates #48
Conversation
@@ -0,0 +1,81 @@ | |||
<!doctype html> |
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.
This one's just a mock so far. It won't be called in the current workflow.
import { Uri, l10n } from 'vscode'; | ||
import { InstructionsWebviewProvider } from '../../webviews/instructions'; | ||
|
||
export class LwcGenerationCommand { |
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.
Another one that's just stubbed. Not accessible to the user yet.
const landingPageLabelId = `${landingPageType}LandingPageTemplateLabel`; | ||
const landingPageLabelElement = | ||
document.getElementById(landingPageLabelId); | ||
if (landingPageStatus.warning) { |
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 not understanding what this is for as I dont see any warning text defined anywhere?
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.
The warning data comes back as one of the status data items from the TemplateChooserCommand.getLandingPageStatus()
method. That status check will create a warning if one or both of the landing page files for the given template are not available in the project.
</div> | ||
</label> | ||
|
||
<label id="defaultLandingPageTemplateLabel"> |
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.
So adding new landing pages in the future will require copy-pasting the html snippets here in this file?
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. But it's a good point: all of these entries could be dynamically generated based on returned data—which they already get, to drive their visibility.
I had thought about that approach, and at the end of the day wasn't sure how much the entries in this page would fluctuate, to make it worthwhile to drive the content dynamically. We're well positioned to do so though, if we decide to go that route.
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.
Some questions, but LGTM
<h1>Create SObject LWC Quick Actions</h1> | ||
|
||
<p> | ||
The following sObjects are present in your configured landing page: |
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.
nit: Probably standardize on SObjects? I'm not sure which capitalization is actually preferred but SObject seems to be used throughout.
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 had done it as a distinction between title case (headings) and lower case (in sentences). But I think for title case, I erroneously looked at the SObject
class in the Apex Reference Guide for guidance, which is likely taking casing cues based more on coding standards than nomenclature.
The Use sObjects trail in Trailhead seems to treat it as "sObject" in all cases. So that's probably the one worth standardizing on. Good callout!
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.
LGTM!
Allows the user to choose their landing page from one of the templates in the Offline Starter Kit project.