[doc] Theme generator Preview (Step 2)#2417
[doc] Theme generator Preview (Step 2)#2417Mil4n0r wants to merge 22 commits intotheme-generatorfrom
Conversation
…o Mil4n0r/theme-generator-preview
…react into Mil4n0r/theme-generator-preview
raquelarrojo
requested review from
raquelarrojo
and removed request for
raquelarrojo
…react into Mil4n0r/theme-generator-preview
apps/website/screens/theme-generator/ThemeGeneratorPreviewPage.tsx
Outdated
Show resolved
Hide resolved
| 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?
There was a problem hiding this comment.
Yes, I think I prefer this option, although we must be aware that if it changes in one place, we have to change it here too.
apps/website/screens/theme-generator/ThemeGeneratorPreviewPage.tsx
Outdated
Show resolved
Hide resolved
apps/website/screens/theme-generator/ThemeGeneratorPreviewPage.tsx
Outdated
Show resolved
Hide resolved
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.
The example has been removed, which is OK, but it also needs to be removed from the selectable list.
There was a problem hiding this comment.
Should we include a disclaimer with these examples, explaining that they are just suggestions?
There was a problem hiding this comment.
I am not sure, we could discuss with design how we could show that.
raquelarrojo
left a comment
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.
There was a problem hiding this comment.
It is already medium size (it's the default one).
You are right on the searchable, I would note to improve the search functionality for group selectors to be included as indexes to filter (currently, if we search "Feedback", no results will be shown). This was already done for Sidenav so we could probably reuse some of the logic, not high priority by any means though, just note it as a low prio issue if possible.
Fixed!
There was a problem hiding this comment.
I mean if it should be small size. It seems larger in implementation than in the design.
There was a problem hiding this comment.
You are right sorry! I was only checking height which does not change between sizes.
Fixed!
apps/website/screens/theme-generator/ThemeGeneratorPreviewPage.tsx
Outdated
Show resolved
Hide resolved
apps/website/screens/utilities/theme-generator/previews/components/AvatarPreview.tsx
Outdated
Show resolved
Hide resolved
apps/website/screens/utilities/theme-generator/previews/components/ImagePreview.tsx
Outdated
Show resolved
Hide resolved
| /> | ||
| </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
There was a problem hiding this comment.
Pending confirmation from design.
Mil4n0r
changed the title
|
The required changes to HalstackProvider for inline styles will be added in #2421 instead of this one to prevent changes outside doc. |
3 participants
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.