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

clarify the difference between attributes and properties #3856

Merged
merged 58 commits into from
Nov 5, 2024

Conversation

smahati
Copy link
Contributor

@smahati smahati commented Aug 15, 2024

Description

Changes proposed in this pull request:

  • documentation suggested for the clarification between attributes and properties

Related issue(s)

Fixed #3848

@smahati
Copy link
Contributor Author

smahati commented Aug 15, 2024

not sure if this is supposed to be in the source code somewhere.

@JohannesDoberer JohannesDoberer self-assigned this Aug 16, 2024
Copy link
Contributor

@JohannesDoberer JohannesDoberer left a comment

Choose a reason for hiding this comment

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

LGTM

@smahati
Copy link
Contributor Author

smahati commented Sep 30, 2024

Added snippets for the compound container, if this looks ok I can add snippets to the other API reference as well.

docs/luigi-compound-container-api.md Outdated Show resolved Hide resolved
docs/luigi-compound-container-api.md Outdated Show resolved Hide resolved
docs/luigi-compound-container-api.md Outdated Show resolved Hide resolved
docs/luigi-compound-container-api.md Outdated Show resolved Hide resolved
docs/luigi-compound-container-api.md Outdated Show resolved Hide resolved
docs/luigi-compound-container-api.md Outdated Show resolved Hide resolved
docs/luigi-compound-container-api.md Outdated Show resolved Hide resolved
docs/luigi-compound-container-api.md Outdated Show resolved Hide resolved
docs/luigi-compound-container-api.md Outdated Show resolved Hide resolved
```

```HTML
<luigi-container viewUrl="/index.html" compoundConfig='{foo: bar}'></luigi-container>
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if we should provide an example for compould attribute in html

Copy link
Contributor Author

Choose a reason for hiding this comment

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

given the size of it, it seems reasonable to not wanting people to do it this way so I removed it

container/typings/LuigiCompoundContainer.svelte.d.ts Outdated Show resolved Hide resolved
container/typings/LuigiContainer.svelte.d.ts Outdated Show resolved Hide resolved
container/typings/LuigiContainer.svelte.d.ts Outdated Show resolved Hide resolved
container/typings/LuigiContainer.svelte.d.ts Outdated Show resolved Hide resolved
container/typings/LuigiContainer.svelte.d.ts Outdated Show resolved Hide resolved
container/typings/LuigiContainer.svelte.d.ts Outdated Show resolved Hide resolved
container/typings/LuigiContainer.svelte.d.ts Outdated Show resolved Hide resolved
container/typings/LuigiContainer.svelte.d.ts Outdated Show resolved Hide resolved
container/typings/LuigiContainer.svelte.d.ts Outdated Show resolved Hide resolved
container/typings/LuigiContainer.svelte.d.ts Outdated Show resolved Hide resolved
@JohannesDoberer
Copy link
Contributor

After the changes please run npm run docu in /scripts because it generates the docu

smahati and others added 17 commits October 23, 2024 14:29
Co-authored-by: Johannes Doberer <[email protected]>
@smahati
Copy link
Contributor Author

smahati commented Oct 25, 2024

@JohannesDoberer I added the examples to both files and generated the docs, I can clean it up a bit more but it's ready to be reviewed.

Copy link
Contributor

@JohannesDoberer JohannesDoberer left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -1 +1 @@
18
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you add this and removed the 18?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I use nvm and it was causing issues as my v18's minor version was not meeting the minimum requirement. However lts/hydrogen is also v18 but updated to the latest version so I changed it to make it easier but can revert if needed and I can probably update my v18 to the latest version too.

@smahati smahati merged commit 374c003 into main Nov 5, 2024
11 checks passed
@smahati smahati deleted the attribute-vs-property-docu branch November 5, 2024 12:09
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.

Clarify the difference between attribute and property in Container Docu
2 participants