Migrate all checkbox inputs to the `CheckBoxInput` component #16813

Devansu-Yadav · 2023-03-12T16:56:32Z

Description

Migrating to the CheckBoxInput component instead of CheckBox component for consistency.

Related Issue(s)

How to test

Release Notes

Migrate all checkbox inputs to the new CheckBoxInput component for consistency

Documentation

Build Options:

/werft with-werft
Run the build with werft instead of GHA
leeway-no-cache
/werft no-test
Run Leeway with --dont-test

Publish Options

/werft publish-to-npm
/werft publish-to-jb-marketplace

Installer Options

with-ee-license
with-dedicated-emulation
with-ws-manager-mk2
workspace-feature-flags
Add desired feature flags to the end of the line above, space separated

Preview Environment Options:

/werft with-local-preview
If enabled this will build install/preview
/werft with-preview
/werft with-large-vm
/werft with-gce-vm
If enabled this will create the environment on GCE infra
/werft with-integration-tests=all
Valid options are all, workspace, webapp, ide, jetbrains, vscode, ssh

werft-gitpod-dev-com · 2023-03-12T16:56:34Z

annotations in the pull request changed, but user is not allowed to start a job

gtsiolis · 2023-03-13T11:16:08Z

/werft run with-preview

👍 started the job as gitpod-build-migrate-checkbox-component-fork.0
(with .werft/ from main)

Devansu-Yadav · 2023-03-20T10:59:32Z

@gtsiolis This PR is ready for review 🏓, although I wasn't able to test out the UX for the recent change I made in this PR as there are some issues while running the dashboard app locally (have already raised this issue on discord and can confirm that it's not because of the changes made in this PR).

Once the UX is good to go, I'll make sure to delete the old CheckBox component.

Note: Some of the changes in this PR may impact the existing UI of some pages already using the new CheckboxInput component, like the Onboarding flow page, which was updated in #16545.

Devansu-Yadav · 2023-03-27T14:52:19Z

@gtsiolis Any updates on whether the UX for the changes made in this PR is good to go or not?

werft-gitpod-dev-com · 2023-03-30T18:04:08Z

started the job as gitpod-build-migrate-checkbox-component.5 because the annotations in the pull request description changed
(with .werft/ from main)

gtsiolis · 2023-03-30T18:04:21Z

/werft run with-preview

👍 started the job as gitpod-build-migrate-checkbox-component-fork.1
(with .werft/ from main)

gtsiolis · 2023-03-30T18:20:04Z

Looking at this now! 👀

gtsiolis

Thanks for your patience and pushing this forward, @Devansu-Yadav! 🌮 🌮

UX LGTM! 🏁

💡 tip: I've deployed up a preview environment in https://migrate-ch7cf88b5c76.preview.gitpod-dev.com/workspaces.

1️⃣ Left some minor comments below.

2️⃣ There's one more checkbox that was added recently, posting the lines of code here in case you'd like to address this change, too.

gitpod/components/dashboard/src/teams/TeamSettings.tsx

Lines 149 to 161 in 58d8381

    
           <CheckBox 
        
               title={<span>Workspace Sharing</span>} 
        
               desc={ 
        
                   <span> 
        
                       Allow workspaces creаted within an Organization to share the workspace with any authenticated user. 
        
                   </span> 
        
               } 
        
               checked={!settings?.workspaceSharingDisabled} 
        
               onChange={({ target }) => 
        
                   handleUpdateTeamSettings({ workspaceSharingDisabled: !target.checked }) 
        
               } 
        
               disabled={isLoading} 
        
           />

3️⃣ Once all Checkbox components have been replaced, we can remove that file and component. It should be fine to do this separately if you prefer. Your call. 🏓

@selfcontained could you take a diagonal look at the code changes here? 🏀

components/dashboard/src/components/forms/CheckboxInputField.tsx

gtsiolis · 2023-03-30T18:27:52Z

components/dashboard/src/components/forms/CheckboxInputField.tsx

+    label: string | React.ReactNode;
+    hint?: string | React.ReactNode;


thought: Not sure about this change, but let me loop in @selfcontained who initially added this component.

This change is made to accommodate for the usage of <CheckBox /> component in certain places (like ProjectSettings) where React nodes are passed as props to specific attributes instead of text.

Makes sense, thanks! We can simplify it to just the React.ReactNode type as that allows for the string type as well.

components/dashboard/src/components/forms/CheckboxInputField.tsx

gtsiolis · 2023-03-30T18:36:24Z

components/dashboard/src/admin/UserDetail.tsx

+                    label="Edit user permissions by adding or removing roles for this user."
+                    className="mt-0"
+                >
+                    {flags.map((e) => (


thought: Highlighting this part for other reviewers. 🏓

This should be rop.map(... so it shows the roles, not flags.

Devansu-Yadav · 2023-03-31T07:32:21Z

💡 tip: I've deployed up a preview environment in https://migrate-ch7cf88b5c76.preview.gitpod-dev.com/workspaces.

Thanks for this!

1️⃣ Left some minor comments below.

Sure, I have left a comment to explain those changes :)

2️⃣ There's one more checkbox that was added recently, posting the lines of code here in case you'd like to address this change, too.

Sure, I have pushed a commit for this.

gitpod/components/dashboard/src/teams/TeamSettings.tsx

Lines 149 to 161 in 58d8381

<CheckBox

title={<span>Workspace Sharing</span>}

desc={

<span>

Allow workspaces creаted within an Organization to share the workspace with any authenticated user.

</span>

}

checked={!settings?.workspaceSharingDisabled}

onChange={({ target }) =>

handleUpdateTeamSettings({ workspaceSharingDisabled: !target.checked })

}

disabled={isLoading}

/>

3️⃣ Once all Checkbox components have been replaced, we can remove that file and component. It should be fine to do this separately if you prefer. Your call. 🏓

@gtsiolis Sure, I would also prefer to remove the old CheckBox component files in a separate PR.

gtsiolis · 2023-03-31T07:33:21Z

I would also prefer to remove the old CheckBox component files in a separate PR.

DEAL—Sounds great, @Devansu-Yadav! No need to do this here. Opened #17103 to keep track of this. 🍊

gtsiolis

UX LGTM. 🌮 🌮

Approving to unblock merging, holding to let someone from the @gitpod-io/engineering-webapp team to take a closer look at the code changes. 🏓

svenefftinge

Code LGTM

gtsiolis · 2023-04-06T12:10:21Z

Thanks for taking a look, @svenefftinge. 🌮 🌮

Removing hold and starting a build from the workspace.

/unhold

svenefftinge · 2023-04-06T12:45:39Z

/werft run

👍 started the job as gitpod-build-migrate-checkbox-component-fork.2
(with .werft/ from main)

aledbf · 2023-04-07T11:01:55Z

/hold

(blocking the merge queue)

gtsiolis · 2023-04-07T14:53:21Z

/werft run with-preview

👍 started the job as gitpod-build-migrate-checkbox-component-fork.3
(with .werft/ from main)

selfcontained

Thank you so much for helping with these changes 😄 I left a few pieces of feedback that would great to address as part of this before we merge it.

selfcontained · 2023-04-10T16:14:13Z

components/dashboard/src/components/forms/InputFieldHint.tsx

@@ -14,7 +14,7 @@ export const InputFieldHint: FC<Props> = ({ disabled = false, children }) => {
    return (
        <span
            className={classNames(
-                "text-sm",
+                "text-md",


I see why this change was made, but I don't think we want to bump the input field hint font size across the board. Having it as text-sm is intentional, especially noticeable our other input fields where we want it to be a slightly smaller font size.

For now can we keep this as text-sm and if we decided we'd like larger text specifically for checkboxes we can either add a property to InputFieldHint for that variance, or not use InputFieldHint on the checkbox component. Let's leave that as a followup and keep it as text-sm please though.

selfcontained · 2023-04-10T18:11:30Z

components/dashboard/src/admin/BlockedRepositories.tsx

+
+            <CheckboxInputContainer>
+                <CheckboxInput
+                    value="Block Users"


Could we make value optional on CheckboxInput, and then we can leave it off in cases like this where it isn't beneficial to set.

@selfcontained Sure, I'll make the change. Is there anywhere else where I should drop the value prop?

I think you should be safe to drop the value prop from all the new places in this PR where we've swapped to the new component if they weren't using it before (I think it's all of them).

selfcontained · 2023-04-10T18:17:42Z

components/dashboard/src/admin/UserDetail.tsx

+                    label="Edit user permissions by adding or removing roles for this user."
+                    className="mt-0"
+                >
+                    {flags.map((e) => (


This should be rop.map(... so it shows the roles, not flags.

components/dashboard/src/components/forms/CheckboxInputField.tsx

Devansu-Yadav · 2023-04-11T15:25:28Z

Thank you so much for helping with these changes 😄 I left a few pieces of feedback that would great to address as part of this before we merge it.

@selfcontained I have addressed almost all the changes you suggested. Do let me know if I missed something 😄

selfcontained · 2023-04-11T22:26:24Z

Thanks @Devansu-Yadav for all your help here 😄

I'm going to update this PR to merge into a non-main branch (bmh/devansu-migrate-checkbox-component) in this repo so that we can work around some things with our build pipeline that we're working through still. I'll go ahead and make the remaining adjustments (removing value from a few more spots), and then we'll get this into main.

Update: New PR w/ some tweaks here.

* Migrate all checkbox inputs to the `CheckBoxInput` component (#16813) * [dashboard] migrate to CheckBoxInput (#16768) * [dashboard] remove CheckBox comp usage * [dashboard] remove `CheckBox` on team settings * [dashboard] remove border on checkbox click state * [dashboard] refactor code for `CheckboxInput` * removing values on checkboxes that don't need it * adjusting api a bit * simplify a bit by always setting value --------- Co-authored-by: Devansu Yadav <[email protected]>

Devansu-Yadav · 2023-04-12T14:15:52Z

Update: New PR w/ some tweaks here.

@selfcontained Just went through this PR and the code looks a whole lot better now! Thanks for the tweaks 🙌

Devansu-Yadav requested a review from a team March 12, 2023 16:56

Devansu-Yadav requested a review from gtsiolis as a code owner March 12, 2023 16:56

roboquat added release-note size/L labels Mar 12, 2023

Devansu-Yadav marked this pull request as draft March 12, 2023 16:57

roboquat added the do-not-merge/work-in-progress label Mar 12, 2023

Devansu-Yadav changed the title ~~[dashboard] migrate to CheckBoxInput (#16768)~~ Migrate all checkbox inputs to the CheckBoxInput component Mar 12, 2023

gtsiolis added the community-contribution label Mar 13, 2023

gtsiolis force-pushed the migrate-checkbox-component branch from 26c115b to fcbb42a Compare March 16, 2023 13:13

Devansu-Yadav force-pushed the migrate-checkbox-component branch from fcbb42a to d7adb40 Compare March 18, 2023 11:13

roboquat added size/XL and removed size/L labels Mar 18, 2023

Devansu-Yadav marked this pull request as ready for review March 20, 2023 10:15

roboquat removed the do-not-merge/work-in-progress label Mar 20, 2023

Devansu-Yadav force-pushed the migrate-checkbox-component branch from f9b7a2a to 7e2e8bd Compare March 20, 2023 10:31

Devansu-Yadav force-pushed the migrate-checkbox-component branch 2 times, most recently from 64e7560 to 8f7037a Compare March 25, 2023 13:56

gtsiolis force-pushed the migrate-checkbox-component branch from 8f7037a to 3be4f04 Compare March 30, 2023 17:56

gtsiolis suggested changes Mar 30, 2023

View reviewed changes

gtsiolis requested a review from selfcontained March 30, 2023 18:44

Devansu-Yadav force-pushed the migrate-checkbox-component branch from 3be4f04 to b968bd1 Compare March 31, 2023 06:42

gtsiolis mentioned this pull request Mar 31, 2023

Remove the obsolete checkbox component #17103

Closed

gtsiolis approved these changes Mar 31, 2023

View reviewed changes

gtsiolis added the do-not-merge/hold label Mar 31, 2023

Devansu-Yadav force-pushed the migrate-checkbox-component branch from d7d04d2 to 50aaad4 Compare April 4, 2023 06:16

svenefftinge approved these changes Apr 6, 2023

View reviewed changes

gtsiolis removed the request for review from selfcontained April 6, 2023 12:08

roboquat removed the do-not-merge/hold label Apr 6, 2023

gtsiolis force-pushed the migrate-checkbox-component branch from 50aaad4 to 7914735 Compare April 6, 2023 12:11

roboquat added the do-not-merge/hold label Apr 7, 2023

selfcontained suggested changes Apr 10, 2023

View reviewed changes

Devansu-Yadav added 4 commits April 11, 2023 14:10

[dashboard] migrate to CheckBoxInput (gitpod-io#16768)

ac4356c

[dashboard] remove CheckBox comp usage

6770e20

[dashboard] remove CheckBox on team settings

8710198

[dashboard] remove border on checkbox click state

bc10b61

Devansu-Yadav force-pushed the migrate-checkbox-component branch from 7914735 to bc10b61 Compare April 11, 2023 14:10

[dashboard] refactor code for CheckboxInput

d1c62ab

gtsiolis requested a review from selfcontained April 11, 2023 15:28

selfcontained changed the base branch from main to bmh/devansu-migrate-checkbox-component April 11, 2023 22:22

selfcontained merged commit 4ac6332 into gitpod-io:bmh/devansu-migrate-checkbox-component Apr 11, 2023

selfcontained mentioned this pull request Apr 11, 2023

Migrate checkbox component #17171

Merged

13 tasks

Devansu-Yadav deleted the migrate-checkbox-component branch April 12, 2023 14:42

Devansu-Yadav mentioned this pull request Apr 12, 2023

Delete the obsolete CheckBox component #17186

Merged

13 tasks

	<CheckBox
	title={<span>Workspace Sharing</span>}
	desc={
	<span>
	Allow workspaces creаted within an Organization to share the workspace with any authenticated user.
	</span>
	}
	checked={!settings?.workspaceSharingDisabled}
	onChange={({ target }) =>
	handleUpdateTeamSettings({ workspaceSharingDisabled: !target.checked })
	}
	disabled={isLoading}
	/>

		label: string \| React.ReactNode;
		hint?: string \| React.ReactNode;

Migrate all checkbox inputs to the CheckBoxInput component #16813

Migrate all checkbox inputs to the CheckBoxInput component #16813

Uh oh!

Conversation

Devansu-Yadav commented Mar 12, 2023 • edited by gtsiolis Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issue(s)

How to test

Release Notes

Documentation

Build Options:

Preview Environment Options:

Uh oh!

werft-gitpod-dev-com bot commented Mar 12, 2023

Uh oh!

gtsiolis commented Mar 13, 2023 • edited by werft-gitpod-dev-com bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Devansu-Yadav commented Mar 20, 2023 • edited by werft-gitpod-dev-com bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Devansu-Yadav commented Mar 27, 2023

Uh oh!

werft-gitpod-dev-com bot commented Mar 30, 2023

Uh oh!

gtsiolis commented Mar 30, 2023 • edited by werft-gitpod-dev-com bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gtsiolis commented Mar 30, 2023 • edited by werft-gitpod-dev-com bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gtsiolis left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Devansu-Yadav commented Mar 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gtsiolis commented Mar 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gtsiolis left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

svenefftinge left a comment

Choose a reason for hiding this comment

Uh oh!

gtsiolis commented Apr 6, 2023 • edited by werft-gitpod-dev-com bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

svenefftinge commented Apr 6, 2023 • edited by werft-gitpod-dev-com bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aledbf commented Apr 7, 2023

Uh oh!

gtsiolis commented Apr 7, 2023 • edited by werft-gitpod-dev-com bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

selfcontained left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Migrate all checkbox inputs to the `CheckBoxInput` component #16813

Migrate all checkbox inputs to the `CheckBoxInput` component #16813

Devansu-Yadav commented Mar 12, 2023 •

edited by gtsiolis

Loading

gtsiolis commented Mar 13, 2023 •

edited by werft-gitpod-dev-com bot

Loading

Devansu-Yadav commented Mar 20, 2023 •

edited by werft-gitpod-dev-com bot

Loading

gtsiolis commented Mar 30, 2023 •

edited by werft-gitpod-dev-com bot

Loading

gtsiolis commented Mar 30, 2023 •

edited by werft-gitpod-dev-com bot

Loading

gtsiolis left a comment •

edited

Loading

Devansu-Yadav commented Mar 31, 2023 •

edited

Loading

gtsiolis commented Mar 31, 2023 •

edited

Loading

gtsiolis left a comment •

edited

Loading

gtsiolis commented Apr 6, 2023 •

edited by werft-gitpod-dev-com bot

Loading

svenefftinge commented Apr 6, 2023 •

edited by werft-gitpod-dev-com bot

Loading

gtsiolis commented Apr 7, 2023 •

edited by werft-gitpod-dev-com bot

Loading

Devansu-Yadav commented Apr 11, 2023 •

edited by werft-gitpod-dev-com bot

Loading

selfcontained commented Apr 11, 2023 •

edited

Loading

Devansu-Yadav commented Apr 12, 2023 •

edited by werft-gitpod-dev-com bot

Loading