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

docs: confusing setAssetPath recommendation #9544

Closed
maxpatiiuk opened this issue Jun 8, 2024 · 3 comments
Closed

docs: confusing setAssetPath recommendation #9544

maxpatiiuk opened this issue Jun 8, 2024 · 3 comments
Assignees
Labels
4 - verified Issues that have been released and confirmed resolved. docs Issues relating to documentation updates only. estimate - 2 Small fix or update, may require updates to tests. p - high Issue should be addressed in the current milestone, impacts component or core functionality

Comments

@maxpatiiuk
Copy link
Member

Description

Calcite's get started docs recommend setting this URL for assets:

setAssetPath("https://js.arcgis.com/calcite-components/2.9.0/assets");
// or
defineCustomElements(window, {
  resourcesUrl: "https://js.arcgis.com/calcite-components/2.9.0/assets"
});

This URL is a bit misleading because the assets part at the end of the above URLs is ignored (because it does not end with /)

To reduce confusion, perhaps the documentation should recommend this url instead: https://js.arcgis.com/calcite-components/2.9.0/

NOTE - this is not a breaking change. Explanation:

Under the hood, Calcite calls Stencil's getAssetPath:

export const getAssetPath = (path: string) => {
  const assetUrl = new URL(path, plt.$resourcesUrl$); // plt.$resourcesUrl$ is the result of setAssetPath call()
  return assetUrl.origin !== win.location.origin ? assetUrl.href : assetUrl.pathname;
};

Given this input:

new URL("./assets/icon/i360View24.json", "https://js.arcgis.com/calcite-components/2.9.0/assets");

The end result is:

https://js.arcgis.com/calcite-components/2.9.0/assets/icon/i360View24.json

See that the /assets/ part is present only once in the final URL, despite being present in the string on the left and on the right.
This is because the new URL()'s 2nd argument does not end in /, which leads to the last part (assets) being ignored.
See MDN documentation: https://developer.mozilla.org/en-US/docs/Web/API/URL/URL#:~:text=When%20specify%20a,a%20URL.

So, it works right now, but it's confusing. I would recommend the documentation be changed to clearly show a URL like this: https://js.arcgis.com/calcite-components/2.9.0/

Which Component

all

Resources

No response

@maxpatiiuk maxpatiiuk added docs Issues relating to documentation updates only. 0 - new New issues that need assignment. needs triage Planning workflow - pending design/dev review. labels Jun 8, 2024
@geospatialem geospatialem self-assigned this Dec 3, 2024
@geospatialem geospatialem added 1 - assigned Issues that are assigned to a sprint and a team member. estimate - 2 Small fix or update, may require updates to tests. p - high Issue should be addressed in the current milestone, impacts component or core functionality and removed 0 - new New issues that need assignment. needs triage Planning workflow - pending design/dev review. labels Dec 3, 2024
@geospatialem
Copy link
Member

geospatialem commented Dec 12, 2024

Summarizing discussions with @jcfranco and @DitwanP on Thurs, Dec 12. Feel free to comment if I missed any additionals 👍🏻:

  • Update the framework examples (refer to this link with the content)
  • Update the Choose a build section on the doc site
    • Simplify the default, which points to the CDN
    • If not using the CDN, the user will need to specify the assets location
  • Update the VS IntelliSense location to the location mentioned in the Lumina PR under Path of extras will change to the following section
  • We may need to update the Component on ready section, in the event we don't need to first specify whenDefined Confirmed that this is still needed with @jcfranco

Additional resources:

@geospatialem geospatialem assigned DitwanP and unassigned geospatialem Dec 12, 2024
@DitwanP DitwanP added 2 - in development Issues that are actively being worked on. and removed 1 - assigned Issues that are assigned to a sprint and a team member. labels Jan 7, 2025
@DitwanP DitwanP added 3 - installed Issues that have been merged to master branch and are ready for final confirmation. and removed 2 - in development Issues that are actively being worked on. labels Feb 4, 2025
Copy link
Contributor

github-actions bot commented Feb 4, 2025

Installed and assigned for verification.

@DitwanP
Copy link
Contributor

DitwanP commented Feb 5, 2025

Verified

Image

@DitwanP DitwanP closed this as completed Feb 5, 2025
@DitwanP DitwanP added 4 - verified Issues that have been released and confirmed resolved. and removed 3 - installed Issues that have been merged to master branch and are ready for final confirmation. labels Feb 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4 - verified Issues that have been released and confirmed resolved. docs Issues relating to documentation updates only. estimate - 2 Small fix or update, may require updates to tests. p - high Issue should be addressed in the current milestone, impacts component or core functionality
Projects
None yet
Development

No branches or pull requests

3 participants