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.

Sign up for GitHub

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

Jump to bottom

V2 #239

Open
wants to merge 28 commits into
base: main
Choose a base branch
from
Open

V2 #239

wants to merge 28 commits into from

Conversation

keithamus
Copy link
Member

@keithamus keithamus commented May 16, 2022 •
edited
Loading

@keithamus keithamus force-pushed the v2 branch 7 times, most recently from bd3c976 to d01d896 Compare May 20, 2022 16:41
@keithamus keithamus force-pushed the v2 branch 6 times, most recently from 173d1b5 to 76bb1d4 Compare May 26, 2022 12:11
@keithamus keithamus marked this pull request as ready for review June 6, 2022 17:36
@keithamus keithamus requested a review from a team as a code owner June 6, 2022 17:36
@keithamus keithamus requested a review from dgreif June 6, 2022 17:36
@keithamus
Copy link
Member Author

I've released a 2.0.0-alpha of this branch: https://github.com/github/catalyst/releases/tag/v2.0.0-alpha1. We'll test this out and kick the tires on it for a while before merging.

@keithamus keithamus requested a review from koddsson as a code owner September 22, 2022 15:13
@keithamus keithamus requested a review from a team as a code owner October 10, 2022 15:47
@primer-css
Copy link

👋 Hello and thanks for pinging us! This issue or PR has been added to our inbox and a Design Infrastructure first responder will review it soon.

  • 🎨 If this is a PR that includes a visual change, please make sure to add screenshots in the description or deploy this code to a lab machine with instructions for how to test.
  • ⏩ If this is a PR that includes changes to an interaction, please include a video recording in the description.
  • ⚠️ If this is urgent, please visit us in #primer on Slack and tag the first responders listed in the channel topic.

@keithamus keithamus requested review from a team and removed request for a team and dgreif October 12, 2022 13:49
@keithamus keithamus mentioned this pull request Oct 12, 2022
Merged
keithamus and others added 22 commits October 12, 2022 15:13
@keithamus
improve your-first-component docs around suffixes
dbda5d0
@keithamus
add exact example for tag name in your-first-component
5ee8fd5
@keithamus
drop the the
b76ac7d
@keithamus
remove autoshadowroot
ec70eda
@keithamus
ensure register tests extend HTMLElement
52d3d40
@keithamus
refactor attr into attrable
b76529a
@keithamus
refactor bind into actionable
af53b8f
@keithamus
rename parse to parseActionAttribute
67f4341
@keithamus
bump size to 2.6kb
d142b92
@keithamus
add new attr docs
21c81fb
@keithamus
refactor target into targetable
bb32c3b
@keithamus
use targetable in controller
d79fc59
@keithamus
fix register type
b7fa0ae
@keithamus
drop size limit to 2.5kb
5f60201
@keithamus
improve @target docs around not using decorators
ab99a16
@keithamus
refine docs around your first component and how controller works
bdb1240
@keithamus
add composition tests for providable
01348fa
@keithamus
export all v2 functions
dfbedf6
@drwsmth @keithamus
Update attrs.md
1409b84 
The current example throws error TS1207: Decorators cannot be applied to multiple get/set accessors of the same name.

I suggest updating documentation inline with advice from TS dev @mhegazy; see microsoft/TypeScript#2249 (comment)
@keithamus @koddsson
Add deprecatedDataPrefixedAttrs decorator
d6c8d14 
This allows classes to opt-in to old-style `data-` prefixed attr
serialised names.

Co-authored-by: Kristján Oddsson <[email protected]>
@keithamus
3kb!
2a0fef3
@keithamus
use shadowroot.host instead of weakmap
5c7c6b0
keithamus and others added 3 commits October 12, 2022 15:14
@keithamus
revert changes to v1 docs
ac03dab
@koddsson @keithamus
Add tests to validate that properties update synchronously
a874af9 
Co-authored-by: Keith Cirkel <[email protected]>
@ryecroft
Add failing test for default-valued properties
5f8d9da 
@attrable properties with a default value specified using a getter
method incorrectly have their setter method called with an empty
string.

This incorrect behaviour does not happen if:

- the property is in the html definition
- the property does not have a default value supplied by a getter method
@adoublef
Copy link

adoublef commented Sep 8, 2023 •
edited
Loading

I am unsure what the status is with v2 so I understand that it's still unstable and subject to changes. I am however finding in comparison to v1 that I am unable to use the target decorator to find the an element that is inside a shadowRoot

Hopefully I am not speaking out of turn with this. But I think the issue was due to the order in with the abilities are called in the in the controller decorator. Seems when I change the function body to register(targetable(actionable(attrable((Class))))); 

In the targetable.ts file it set's the shadowRoot to the weakmap which I think was not being called (or overwritten) with the original controller decorator

@keithamus
Merge pull request #297 from ryecroft/data_properties
d352596 
Add failing test for default-valued properties
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@colebemis colebemis colebemis approved these changes

@koddsson koddsson Awaiting requested review from koddsson

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@keithamus @primer-css @adoublef @colebemis @drwsmth @koddsson @ryecroft