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

chore(web): add useManageSwitchState #471

Merged
merged 4 commits into from Jun 9, 2023

Conversation

atnagata
Copy link
Contributor

@atnagata atnagata commented Jun 6, 2023

Overview

add useManageSwitchState

How I tested

Confirmation of activation assuming multiple component (button) state use

@atnagata atnagata requested a review from KaWaite as a code owner June 6, 2023 08:36
@netlify
Copy link

netlify bot commented Jun 6, 2023

Deploy Preview for reearth-web ready!

Name Link
🔨 Latest commit 7d8dc84
🔍 Latest deploy log https://app.netlify.com/sites/reearth-web/deploys/64827afa1a51bc00080d89a0
😎 Deploy Preview https://deploy-preview-471--reearth-web.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 Jun 6, 2023

Codecov Report

Merging #471 (7d8dc84) into main (24d6c39) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #471      +/-   ##
==========================================
+ Coverage   26.72%   26.73%   +0.01%     
==========================================
  Files        1279     1280       +1     
  Lines      141228   141253      +25     
  Branches     3436     3440       +4     
==========================================
+ Hits        37738    37763      +25     
  Misses     102369   102369              
  Partials     1121     1121              
Flag Coverage Δ
web 24.02% <100.00%> (+0.01%) ⬆️
web-beta 24.02% <100.00%> (+0.01%) ⬆️
web-classic 24.02% <100.00%> (+0.01%) ⬆️
web-utils 24.02% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
web/src/beta/hooks/useManageSwitchState/hooks.tsx 100.00% <100.00%> (ø)

Comment on lines 7 to 8
id: string;
active: boolean;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
id: string;
active: boolean;
name: string;

I think the props has id and active field in default, so could you set name to this field?
Also could you check if the name field is included in the fields state which is returned from the hooks?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added and committed.

@@ -2,6 +2,7 @@ import { useState, useCallback } from "react";

type SwitchField<T> = {
id: string;
name?: string;
Copy link
Member

Choose a reason for hiding this comment

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

これはいらないです

Copy link
Member

@KaWaite KaWaite left a comment

Choose a reason for hiding this comment

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

After @keiya01 's comment is addressed and resolved you can merge!

@keiya01 keiya01 merged commit 97f7f2f into reearth:main Jun 9, 2023
16 of 17 checks passed
koji-endo2 pushed a commit to atnagata/reearth that referenced this pull request Jun 9, 2023
Co-authored-by: 長田淳史 <>
Co-authored-by: keiya sasaki <keiya.s.0210@gmail.com>
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.

None yet

3 participants