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

Add text regarding choosing between events and observable properties #2018

Merged
merged 2 commits into from
May 28, 2024

Conversation

JKRhb
Copy link
Member

@JKRhb JKRhb commented May 15, 2024

Resolves #1818.


Preview | Diff

@JKRhb
Copy link
Member Author

JKRhb commented May 15, 2024

I was a bit unsure where to put the explanation – so far, I added it to the bottom of the section on EventAffordances, but this could maybe also become part of an appendix or a more general section on best practices for TD design.

Copy link
Contributor

@egekorkan egekorkan left a comment

Choose a reason for hiding this comment

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

I think that the text addition is good. I have proposed a further distinction.

Regarding your question " or a more general section on best practices for TD design.". I feel that we have similar stuff in multiple places. Going forward, I think we can think of adding some tags for HTML elements that can be best practices. Like the implementation report, we can extract such best practices as a separate document for implementers. It can be a separate section but I fear that many texts can be too detached from their context. Definitely something to discuss in a call but the PR itself is fine.

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Show resolved Hide resolved
@JKRhb JKRhb marked this pull request as ready for review May 16, 2024 19:53
@egekorkan
Copy link
Contributor

The PR is fine, will be merged async after rebasing

@JKRhb JKRhb force-pushed the event-vs-observable branch 2 times, most recently from 3a47395 to 1ee9b68 Compare May 22, 2024 16:26
@JKRhb
Copy link
Member Author

JKRhb commented May 22, 2024

Hmm, for some reason, the version I generate locally is now even more different from the main branch branch than before :/ To simplify things, I would simply remove my regeneration commit, and we could then have a clean version generated on the main branch, for example.

@egekorkan
Copy link
Contributor

I have merged my latest pr so maybe that messed it up. If you want, please create a new PR with the same changes and we can merge async

@JKRhb
Copy link
Member Author

JKRhb commented May 23, 2024

I have merged my latest pr so maybe that messed it up. If you want, please create a new PR with the same changes and we can merge async

Hmm, do you mean a PR that regenerates the current state on the main branch? Otherwise, I could also “wrap” the commit that adds the text to the .ttl file in between two regeneration commits, the latter of which would illustrate the changes that would be added to the main document.

@egekorkan
Copy link
Contributor

Ok I have understood the problem. Basically, if you render, the index.html generated does not follow our prettify rules so it overwrites the previous formatting. Once I ran formatting after rendering, the unwanted changes were gone. I hope that we will not have these small but annoying issues with new tooling.

@egekorkan egekorkan merged commit 67b7c91 into w3c:main May 28, 2024
2 checks passed
@JKRhb JKRhb deleted the event-vs-observable branch May 28, 2024 12:20
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.

Better explain difference between observable properties and events
2 participants