Skip to content

#625: <option> tag should also be valueOnly#628

Closed
philiptzou wants to merge 1 commit intoremarkablemark:masterfrom
philiptzou:patch-1
Closed

#625: <option> tag should also be valueOnly#628
philiptzou wants to merge 1 commit intoremarkablemark:masterfrom
philiptzou:patch-1

Conversation

@philiptzou
Copy link
Copy Markdown

@philiptzou philiptzou commented Jul 11, 2022 •
edited by remarkablemark
Loading

Fixes #625

Checklist:

remarkablemark
remarkablemark requested changes Jul 11, 2022
View reviewed changes
Copy link
Copy Markdown
Owner

@remarkablemark remarkablemark left a comment

Choose a reason for hiding this comment

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

Thanks for opening the PR @philiptzou!

Do you mind amending your commit message so it passes the linter?

git commit --amend -m "fix: option tag should also be valueOnly" -m "Fixes #625"
git push -f

Also, do you mind adding a test?

@remarkablemark remarkablemark added the bug Something isn't working label Jul 11, 2022
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jul 11, 2022

Codecov Report

Merging #628 (829de65) into master (d7ba9d7) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master      #628   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            8         8           
  Lines          167       167           
  Branches        57        57           
=========================================
  Hits           167       167
Impacted Files Coverage Δ
lib/attributes-to-props.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d7ba9d7...829de65. Read the comment docs.

@philiptzou
Copy link
Copy Markdown
Author

philiptzou commented Jul 11, 2022

@remarkablemark I think this may need more works since I didn't test it. I'll work on this tomorrow.

@remarkablemark
Copy link
Copy Markdown
Owner

remarkablemark commented Jul 11, 2022

@remarkablemark I think this may need more works since I didn't test it. I'll work on this tomorrow.

Sounds good 👍

@remarkablemark
Copy link
Copy Markdown
Owner

remarkablemark commented Jul 30, 2022

Hey @philiptzou, were you able to make any progress? Let me know if you need any help

@remarkablemark
Copy link
Copy Markdown
Owner

remarkablemark commented Jan 5, 2023 •
edited
Loading

Closed by #790

Release:

@remarkablemark remarkablemark mentioned this pull request Jan 5, 2023
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@remarkablemark remarkablemark remarkablemark requested changes

Assignees

No one assigned

Labels

bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

value= shouldn't be converted to defaultValue= for <option>

3 participants

@philiptzou @codecov-commenter @remarkablemark