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.

Sign up for GitHub

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

note added for public and private environmets #119

Merged
merged 1 commit into from
Mar 5, 2024
Merged

note added for public and private environmets #119

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 18 additions & 30 deletions src/apps/console/page-components/new-scope.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import Yup from '~/root/lib/server/helpers/yup';
import { handleError } from '~/root/lib/utils/common';
import { Switch } from '~/components/atoms/switch';
import { Checkbox } from '~/components/atoms/checkbox';
import { InfoLabel } from '~/console/components/commons';
import Banner from '~/components/molecule/banner';
import { IDialog } from '../components/types.d';
import { useConsoleApi } from '../server/gql/api-provider';
import { DIALOG_TYPE } from '../utils/commons';
Expand Down Expand Up @@ -149,35 +149,23 @@ const HandleScope = ({ show, setShow }: IDialog<IEnvironment | null>) => {
nameErrorLabel="isNameError"
isUpdate={show?.type !== DIALOG_TYPE.ADD}
/>
<div className="flex flex-row items-center gap-lg">
<Checkbox
label="Public"
checked={values.environmentRoutingMode}
onChange={(val) => {
handleChange('environmentRoutingMode')(dummyEvent(val));
}}
/>
<InfoLabel
info={
<div className="flex flex-col gap-2xl">
<div>
<div className="bodyMd-medium">Public</div>
<p>
Public environments will expose services to the public
internet.
</p>
</div>
<div>
<div className="bodyMd-medium">Private</div>
<p>
Private environments will be accessible when Kloudlite
VPN is active.
</p>
</div>
</div>
}
/>
</div>
<Checkbox
label="Public"
checked={values.environmentRoutingMode}
onChange={(val) => {
handleChange('environmentRoutingMode')(dummyEvent(val));
}}
/>
<Banner
Copy link

Choose a reason for hiding this comment

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

question (llm): Given the introduction of the Banner component to convey information about the public and private environment settings in the new scope creation flow, it's essential to validate that this information is presented correctly to the user. Testing for the presence and correctness of the Banner content in this context would be beneficial. If such tests are not yet implemented, I suggest adding them to ensure the feature's integrity and to prevent any misinformation.

type="info"
body={
<span>
Public environments will expose services to the public
internet. Private environments will be accessible when
Kloudlite VPN is active.
</span>
}
/>
</div>
</Popup.Content>
<Popup.Footer>
Expand Down
47 changes: 18 additions & 29 deletions src/apps/console/routes/_main+/$account+/$project+/$environment+/settings+/general/route.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { useReload } from '~/root/lib/client/helpers/reloader';
import Wrapper from '~/console/components/wrapper';
import { Checkbox } from '~/components/atoms/checkbox';
import { InfoLabel } from '~/console/components/commons';
import Banner from '~/components/molecule/banner';
import { IEnvironmentContext } from '../../_layout';

const EnvironmentSettingsGeneral = () => {
Expand Down Expand Up @@ -129,35 +130,23 @@ const EnvironmentSettingsGeneral = () => {
value={values.displayName}
onChange={handleChange('displayName')}
/>
<div className="flex flex-row items-center gap-lg">
<Checkbox
label="Public"
checked={values.environmentRoutingMode}
onChange={(checked) => {
handleChange('environmentRoutingMode')(dummyEvent(checked));
}}
/>
<InfoLabel
info={
<div className="flex flex-col gap-2xl">
<div>
<div className="bodyMd-medium">Public</div>
<p>
Public environments will expose services to the public
internet.
</p>
</div>
<div>
<div className="bodyMd-medium">Private</div>
<p>
Private environments will be accessible when Kloudlite VPN
is active.
</p>
</div>
</div>
}
/>
</div>
<Checkbox
label="Public"
checked={values.environmentRoutingMode}
onChange={(checked) => {
handleChange('environmentRoutingMode')(dummyEvent(checked));
}}
/>
Copy link

Choose a reason for hiding this comment

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

suggestion (llm): Replacing the detailed InfoLabel with a Banner for displaying environment access information simplifies the UI. However, ensure that the condensed information in the Banner is sufficient for users to understand the implications of public vs. private environments without the need for additional context.

<Banner
Copy link

Choose a reason for hiding this comment

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

question (llm): I noticed the introduction of a Banner component to display information about public and private environments. While this change seems primarily UI-focused, it's important to ensure that the information displayed remains accurate and accessible to users. Could you confirm if there are any existing tests that verify the content of this banner, especially since it contains critical information about environment accessibility? If not, adding a test to verify the banner's content and its visibility based on certain conditions would enhance the reliability of this feature.

type="info"
body={
<span>
Public environments will expose services to the public internet.
Private environments will be accessible when Kloudlite VPN is
active.
</span>
}
/>
<div className="flex flex-row items-center gap-3xl">
<div className="flex-1">
<TextInput
Expand Down
47 changes: 18 additions & 29 deletions src/apps/console/routes/_main+/$account+/$project+/environments/clone-environment.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import useForm, { dummyEvent } from '~/root/lib/client/hooks/use-form';
import Yup from '~/root/lib/server/helpers/yup';
import { handleError } from '~/root/lib/utils/common';
import { InfoLabel } from '~/console/components/commons';
import Banner from '~/components/molecule/banner';

type IDialog = IDialogBase<ExtractNodeType<IEnvironments>>;

Expand Down Expand Up @@ -87,35 +88,23 @@ const Root = (props: IDialog) => {
handleChange={handleChange}
nameErrorLabel="isNameError"
/>
<div className="flex flex-row items-center gap-lg">
<Checkbox
label="Public"
checked={values.environmentRoutingMode}
onChange={(val) => {
handleChange('environmentRoutingMode')(dummyEvent(val));
}}
/>
<InfoLabel
info={
<div className="flex flex-col gap-2xl">
<div>
<div className="bodyMd-medium">Public</div>
<p>
Public environments will expose services to the public
internet.
</p>
</div>
<div>
<div className="bodyMd-medium">Private</div>
<p>
Private environments will be accessible when Kloudlite VPN
is active.
</p>
</div>
</div>
}
/>
</div>
<Checkbox
label="Public"
checked={values.environmentRoutingMode}
onChange={(val) => {
handleChange('environmentRoutingMode')(dummyEvent(val));
}}
/>
<Banner
Copy link

Choose a reason for hiding this comment

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

question (llm): The replacement of the detailed InfoLabel with a Banner component for displaying environment accessibility information is a significant UI change. It's crucial to ensure that the functional aspect of informing users is not compromised. Are there tests in place to check the rendering and content accuracy of this Banner component in the context of cloning an environment? If not, I recommend adding such tests to ensure that users receive the correct information during the cloning process.

type="info"
body={
<span>
Public environments will expose services to the public internet.
Private environments will be accessible when Kloudlite VPN is
active.
</span>
}
/>
</div>
</Popup.Content>
<Popup.Footer>
Expand Down
Loading