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

fix: error on set placeholder partially #269

Merged
merged 3 commits into from
Apr 27, 2023
Merged

Conversation

ryo-gk
Copy link
Member

@ryo-gk ryo-gk commented Apr 26, 2023

This PR fixes the issue that it causes the error when we set the placeholder to SInputYMD and SInputHMS partially.

@ryo-gk ryo-gk added the bug Something isn't working label Apr 26, 2023
@ryo-gk ryo-gk requested a review from kiaking April 26, 2023 04:08
@ryo-gk ryo-gk self-assigned this Apr 26, 2023
@netlify
Copy link

netlify bot commented Apr 26, 2023

Deploy Preview for sefirot-story ready!

Name Link
🔨 Latest commit d872f58
🔍 Latest deploy log https://app.netlify.com/sites/sefirot-story/deploys/64490b58d61c100008c6b582
😎 Deploy Preview https://deploy-preview-269--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 settings.

@netlify
Copy link

netlify bot commented Apr 26, 2023

Deploy Preview for sefirot-docs ready!

Name Link
🔨 Latest commit d872f58
🔍 Latest deploy log https://app.netlify.com/sites/sefirot-docs/deploys/64490b5842610500089823de
😎 Deploy Preview https://deploy-preview-269--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 settings.

@codecov
Copy link

codecov bot commented Apr 26, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (90bb10c) 80.32% compared to head (d872f58) 80.32%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #269   +/-   ##
=======================================
  Coverage   80.32%   80.32%           
=======================================
  Files         114      114           
  Lines        9887     9887           
  Branches      411      411           
=======================================
  Hits         7942     7942           
  Misses       1945     1945           
Impacted Files Coverage Δ
lib/components/SInputHMS.vue 100.00% <100.00%> (ø)
lib/components/SInputYMD.vue 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Member

@kiaking kiaking left a comment

Choose a reason for hiding this comment

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

We need to update the Placeholder types as well since they are not nullable 👀 It should be:

export interface Placeholder {
  hour?: string
  minute?: string
  second?: string
}

@kiaking
Copy link
Member

kiaking commented Apr 26, 2023

Same goes for SInputYMD too 👀

@ryo-gk
Copy link
Member Author

ryo-gk commented Apr 26, 2023

Ah, I mistook the Placeholder in SInputYMD for Value and thought it was okay. 😓
I fixed it.

@kiaking kiaking added enhancement New feature or request and removed bug Something isn't working labels Apr 27, 2023
@kiaking kiaking merged commit cb3b998 into main Apr 27, 2023
13 checks passed
@kiaking kiaking deleted the fix/placeholde-ymd-hms branch April 27, 2023 02:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants