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(action-list): add <ActionList> (#358) #364

Merged
merged 1 commit into from
Nov 1, 2023
Merged

Conversation

kiaking
Copy link
Member

@kiaking kiaking commented Nov 1, 2023

Add <ActionList>. I have made the type simpler than I described on the issue. Well, this is all we need at the moment.

interface Props {
  list?: ActionList
}

type ActionList = ActionListItem[]

interface ActionListItem {
  leadIcon?: IconifyIcon
  text: string
  link?: string
  onClick?(): void
}

Also, I have made few css var adjustment to match the latest color system on Figma. --c-mute is deprecated in favor of new --c-bg-mute-1 series.


Screenshot 2023-11-01 at 16 06 48

@kiaking kiaking requested a review from brc-dd November 1, 2023 07:09
@kiaking kiaking self-assigned this Nov 1, 2023
Copy link

netlify bot commented Nov 1, 2023

Deploy Preview for sefirot-docs ready!

Name Link
🔨 Latest commit 9d5385d
🔍 Latest deploy log https://app.netlify.com/sites/sefirot-docs/deploys/6541f9c112228c00083b1536
😎 Deploy Preview https://deploy-preview-364--sefirot-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Nov 1, 2023

Deploy Preview for sefirot-story ready!

Name Link
🔨 Latest commit 9d5385d
🔍 Latest deploy log https://app.netlify.com/sites/sefirot-story/deploys/6541f9c1507a9b00085d3ea4
😎 Deploy Preview https://deploy-preview-364--sefirot-story.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

codecov bot commented Nov 1, 2023

Codecov Report

Attention: 87 lines in your changes are missing coverage. Please review.

Comparison is base (c536e3d) 82.76% compared to head (9d5385d) 82.15%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #364      +/-   ##
==========================================
- Coverage   82.76%   82.15%   -0.61%     
==========================================
  Files         145      147       +2     
  Lines       11719    11806      +87     
  Branches      655      657       +2     
==========================================
  Hits         9699     9699              
- Misses       2020     2107      +87     
Files Coverage Δ
lib/components/SActionList.vue 0.00% <0.00%> (ø)
lib/components/SActionListItem.vue 0.00% <0.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@brc-dd
Copy link
Member

brc-dd commented Nov 1, 2023

Probably should use markRaw(IconSomething) -- Doesn't seem to be affecting in this case, but mostly Vue will log warning.

@kiaking
Copy link
Member Author

kiaking commented Nov 1, 2023

@brc-dd Ah, yeah. But that only happens when you set icon inside reactive object right...? But that could happen as soon as we pass the object inside template...?

@brc-dd
Copy link
Member

brc-dd commented Nov 1, 2023

Yeah. I'm not sure how reactivity exactly works with arrays passed as props. But if you make them computed or something (like if you want to change that list, add/remove some item), that most likely will throw warnings. I don't think any code change is needed though. Probably can just update the example in docs.

@kiaking
Copy link
Member Author

kiaking commented Nov 1, 2023

Make sense 👍

@kiaking kiaking merged commit dd89f13 into main Nov 1, 2023
9 of 13 checks passed
@kiaking kiaking deleted the 358-action-list branch November 1, 2023 08:08
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.

Add <ActionList>
2 participants