Conversation
…o Mil4n0r/theme-generator-preview
…react into Mil4n0r/theme-generator-preview
…react into Mil4n0r/theme-generator-preview
| }} | ||
| mode="secondary" | ||
| semantic="error" | ||
| disabled={mode === "components" ? selectedComponents.length === 0 : !!selectedExample} |
There was a problem hiding this comment.
The logic of the delete button is inverted in examples mode. When an example is selected, the delete button is disabled instead of enabled (and vice versa).
| import { useMemo, useState } from "react"; | ||
| import componentsList from "../common/componentsList.json"; | ||
| import { componentsRegistry, examplesRegistry } from "screens/utilities/theme-generator/componentsRegistry"; | ||
| import { ListOptionType } from "../../../../packages/lib/src/select/types"; |
There was a problem hiding this comment.
Should this be imported from @dxc-technology/halstack-react ? instead of this cross-package import?.
There was a problem hiding this comment.
We don't currently export types. (We should ideally use something like DefinitelyTyped). For now I can simply repeat the typing in this file, is it okay?
|
|
||
| // TODO: this is just a quick solution to make the preview area scrollable when the content is too big, I don't know what other approach that doesn't | ||
| // involve adding custom styles could be used. (fullHeight does not do anything here either) | ||
| const CustomPreviewArea = styled(DxcFlex)` |
There was a problem hiding this comment.
Instead of overriding the DxcFlex styles, I think it would be better to create a native div element with display: flex; and overflow: hidden;
| return mapToSelectGroups(componentsList as ComponentItem[]); | ||
| }, []); | ||
|
|
||
| const exampleOptions = [ |
There was a problem hiding this comment.
Since this is a static array, could it be moved outside the component?
| @@ -0,0 +1,177 @@ | |||
| import { DxcButton, DxcContainer, DxcFlex, DxcSelect, DxcToggleGroup } from "@dxc-technology/halstack-react"; | |||
| import { useMemo, useState } from "react"; | |||
| import componentsList from "../common/componentsList.json"; | |||
There was a problem hiding this comment.
The layout components should not appear in the component list. Only components affected by theme colours should appear in the selector. 'Bleed', 'Container', 'Flex', 'Grid', 'Inset' and 'Popover' layout components are not visually affected by colour tokens and should not be selectable. The same applies to dialog, card and text components.
There was a problem hiding this comment.
Application and dashboard examples should render the header and footer. If they do not, I think it is not possible to preview the logos, for example.
There was a problem hiding this comment.
Login example doesn't make much sense as a standalone preview in this context.
There was a problem hiding this comment.
Should we include a disclaimer with these examples, explaining that they are just suggestions?
raquelarrojo
left a comment
There was a problem hiding this comment.
I would add some real context to the previews. Avoid "Label", "Sublabel", First step", "Second step" and give real values.
| {mode === "components" && ( | ||
| <DxcSelect | ||
| placeholder="Select components" | ||
| options={componentOptions} |
There was a problem hiding this comment.
Should this select be medium size? Seems to be larger than in design.
I would add searchable functionality to the select to make it easier the selection.
| disabled={mode === "components" ? selectedComponents.length === 0 : !selectedExample} | ||
| /> | ||
| </DxcFlex> | ||
| <CustomPreviewArea>{displayedPreview}</CustomPreviewArea> |
There was a problem hiding this comment.
I thin we should add a message like in design when there is no selected component, like Select a component to preview
| primaryText="John Doe" | ||
| secondaryText="johndoe@company.com" | ||
| status={{ mode: "success", position: "top" }} | ||
| /> |
There was a problem hiding this comment.
I think we should avoid scroll and add the avatar previews in more than one row.
| import { DxcImage } from "@dxc-technology/halstack-react"; | ||
|
|
||
| const ImagePreview = () => { | ||
| return <DxcImage src="https://placehold.co/600x400" alt="Sample image" />; |
There was a problem hiding this comment.
Makes it sense to add the image preview? Can it be customizable?
| /> | ||
| </DxcFlex> | ||
| <CustomPreviewArea>{displayedPreview}</CustomPreviewArea> | ||
| </DxcFlex> |
There was a problem hiding this comment.
I think we could add the Component name as a title above each preview. We can confirm with design but I thinks it would be helpful
Checklist
(Check off all the items before submitting)
/libdirectory./websiteas needed.Added step 2 for theme generator config.
Still need to address the issue with the layout for having scroll inside the preview area. The rest can be reviewed.
Merge step 1 first if possible.