Skip to content
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

feat: add analytics__Dashboard js-meta.xml support, and js-meta bug fixes #3232

Merged
merged 5 commits into from
May 18, 2021

Conversation

smithgp
Copy link
Collaborator

@smithgp smithgp commented May 12, 2021

What does this PR do?

This updates the js-meta.xml schema support:

  • Adds analytics__Dashboard js-meta.xml support, for the new LWC's in analytics dashboard feature. This includes the Dimension and Measure type's in <property>.
  • Updated the js-meta.xml feature's checking of the vscode-xml extension version -- it appears that the bug that broke 0.15.0 got fixed in 0.16.0
  • Adds missing fields: <propertyType>, filter on <property>
  • Adds typing to type on <property>

What issues does this PR fix or reference?

@W-9191640@, @W-8852786@

Functionality Before

  • You would get a schema error in the editor if you used <target>analytics__Dashboard</target> or used <hasStep> in a <targetConfig>.
  • You didn't get code-completion for analytics__Dashboard, nor hover text
  • You would get schema error on <propertyType> and filter.
  • You got a wrong output channel message if you had 0.16.0 of vscode-xml installed, and the schema extensions for js-meta.xml files didn't work.

Functionality After

That all works now.

@smithgp smithgp requested a review from a team as a code owner May 12, 2021 00:42
@smithgp smithgp requested a review from xyc May 12, 2021 00:42
<xs:sequence>
<xs:element type="xs:boolean" name="hasStep" minOccurs="0">
<xs:annotation>
<xs:documentation>Specify that your component requires an attached step to function as expected. Only valid for `analytics__Dashboard` targets. With this set to `true`, the Tableau CRM dashboard builder UI prompts you to attach an existing step or create a new step when creating an instance of your component. Components with an attached step have access to step-specific properties like `results` and `selection`.</xs:documentation>
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

vscode-xml extension handles markdown in xs:documentation in the hovers, and markdown works fine as plain text if any other tool ever looks at this xsd.


const catalogPaths = [
path.join(
['0.14.0', '0.16.0', '1.0.0'].forEach(rhExtensionVersion => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just running the existing tests against different versions -- it's more obvious if you look at this part with whitespace diffs off.

@smithgp smithgp force-pushed the gps/analtyics-dashboard-target branch from ab632a6 to 5e0536f Compare May 12, 2021 00:49
@smithgp smithgp requested review from jag-j and mblumreich May 12, 2021 00:50
@smithgp
Copy link
Collaborator Author

smithgp commented May 12, 2021

cc: @jimmydief

Copy link

@mblumreich mblumreich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added 2 minor edits for doc attributes

@smithgp smithgp force-pushed the gps/analtyics-dashboard-target branch from 12eba1f to 0311510 Compare May 13, 2021 00:17
@jimmydief
Copy link

Thanks @smithgp, this looks good to me so far! The one missing piece is the new "Measure" and "Dimension" attribute types. These are only allowed in target configs with <hasStep>true</hasStep>.

@smithgp
Copy link
Collaborator Author

smithgp commented May 13, 2021

Thanks @smithgp, this looks good to me so far! The one missing piece is the new "Measure" and "Dimension" attribute types. These are only allowed in target configs with <hasStep>true</hasStep>.

Yep, I thought of those, but the schema currently has the <property type> as a plain xs:string right now, with a hovertext to check the docs, which then does the listing out of various values per target. I initially left that as is for now, since there's already several other (unrelated) types that are only valid for certain targets too.
But, looking at the current docs, I think I can fill it out here for Measure & Dimension and the othern property types, with appropriate hovertext.

@smithgp smithgp marked this pull request as draft May 13, 2021 22:08
@smithgp smithgp force-pushed the gps/analtyics-dashboard-target branch from 0311510 to 852a40a Compare May 14, 2021 22:24
@smithgp smithgp requested a review from mblumreich May 14, 2021 22:32
<!-- type attribute on property XSD -->
<xs:simpleType name="propertyTypeAttr">
<xs:union>
<xs:simpleType>
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does give code-completion for these enumerated values, but allows people to type in the valid patterns below.

<xs:restriction base="xs:string">
<xs:enumeration value="Boolean">
<xs:annotation>
<xs:documentation>A boolean value.</xs:documentation>
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to have a documentation annotation here, or the hover text on the attribute value shows the default annotation of the filter attribute itself which looks funny.

</xs:enumeration>
<xs:enumeration value="Color">
<xs:annotation>
<xs:documentation>Displays a color selector. Use the default attribute to specify RGBA, RGB, or hex strings. Supported only if the target is `lightningCommunity__Default`.</xs:documentation>
Copy link
Collaborator Author

@smithgp smithgp May 14, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These descriptions are copied from the public documentation.

@smithgp smithgp marked this pull request as ready for review May 14, 2021 22:35
@smithgp smithgp force-pushed the gps/analtyics-dashboard-target branch from 852a40a to 2a5c346 Compare May 15, 2021 21:48
With how this xsd is currently setup, this is the analytics__Dashboard target
and the hasStep targetConfig tag.
@W-9191640@
Without this fix, the channel message when 0.16.0 is installed is wrong since
the logic here assumed it was 0.13.0 or less.
Also, it looks like the original issue in the vscode-xml extension got fixed
in the 0.16.0 release, and it is working locally for me.
I also made it a little more future proof by also enabling it if/when the
version goes to 1.x+, and updated the tests to check for all of those version.
These are in the docs but were not listed here:
- <propertyType>
- `filter` on <property>
@smithgp smithgp force-pushed the gps/analtyics-dashboard-target branch from 2a5c346 to c1392f5 Compare May 18, 2021 02:05
@smithgp smithgp merged commit 87e0386 into develop May 18, 2021
@smithgp smithgp deleted the gps/analtyics-dashboard-target branch May 18, 2021 03:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants