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

Improve creator UI #57

Merged
merged 1 commit into from
Aug 8, 2024
Merged

Improve creator UI #57

merged 1 commit into from
Aug 8, 2024

Conversation

randomnetcat
Copy link
Contributor

More closely approximate the mockups at RHCLOUD-30247.

Copy link
Contributor

@Hyperkid123 Hyperkid123 left a comment

Choose a reason for hiding this comment

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

The last step should not have the preview enabled. It should also have an action to create a new resource:
screenshot-www_figma_com-2024_07_17-10_24_28

There is also a missing banner.

src/components/creator/CreatorWizard.tsx Show resolved Hide resolved
src/components/creator/steps/details.tsx Show resolved Hide resolved
src/Creator.tsx Outdated Show resolved Hide resolved
src/components/creator/schema.tsx Show resolved Hide resolved
<Title headingLevel="h2" size="xl" className="pf-v5-u-mb-md">
Live card preview
</Title>
<Flex
Copy link
Contributor

Choose a reason for hiding this comment

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

The preview should not have a white background.

screenshot-stage_foo_redhat_com_1337-2024_07_17-10_20_53

Copy link
Contributor Author

@randomnetcat randomnetcat Jul 17, 2024

Choose a reason for hiding this comment

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

It... doesn't on my end? On either Firefox or Chrome
image

What browser are you using? How are you testing it?

Copy link
Contributor

Choose a reason for hiding this comment

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

I use the latest Chrome. I did nothing special, the background is just white.

Copy link
Contributor Author

@randomnetcat randomnetcat Jul 18, 2024

Choose a reason for hiding this comment

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

Running it with npm run build; BETA=true npm run static -- --port=8004 in learning-resources and BETA=true CHROME_SERVICE=8000 CONFIG_PORT=8000 NAV_CONFIG=8000 LOCAL_APPS=learning-resources:8004~http npm run dev in insights-chrome (on the latest master), I still get a grey background on Google Chrome.

The quickstart pulling focus in #57 (review) makes me think at least the quickstarts dependency is outdated (should be 5.4.0-prerelease.1); could you double check that?

Also, (as I asked below) what OS is this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Tested locally and I am not seeing the white background:
image

src/components/creator/CreatorWizard.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

@Hyperkid123 Hyperkid123 left a comment

Choose a reason for hiding this comment

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

For quickstarts:

The tasks filed array should look different:
screenshot-www_figma_com-2024_07_17-10_29_14

Currently, your items lose focus when typing into them. I suspect the components are not re-rendered but re-created instead:
screencast-stage_foo_redhat_com_1337-2024_07_17-10_32_31.webm

The preview of a quickstart is showing extra empty space:
screenshot-stage_foo_redhat_com_1337-2024_07_17-10_33_50

The show work check should be a toggle/switch, not a checkbox:

screenshot-stage_foo_redhat_com_1337-2024_07_17-10_34_49

Copy link
Contributor

@Hyperkid123 Hyperkid123 left a comment

Choose a reason for hiding this comment

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

For learning paths:

The "Associated bundles: select is supposed to be required. Rest is similar to the documentation steps.

For the other category:

Same as associated bundles.

@randomnetcat
Copy link
Contributor Author

randomnetcat commented Jul 17, 2024

Currently, your items lose focus when typing into them. I suspect the components are not re-rendered but re-created instead:

I added a feature to the quickstarts extension to configure it not to grab focus. That should have been pulled in with RedHatInsights/insights-chrome#2870

The preview of a quickstart is showing extra empty space:

This doesn't show up in my local setup.
image
What browser are you using? Is it possible the quickstarts version is different?

@randomnetcat
Copy link
Contributor Author

There is also a missing banner.

That will need data-driven-forms/react-forms#1476 to be merged so that the padding around the wizard body can be removed (and the banner can go to the edges).

@randomnetcat
Copy link
Contributor Author

randomnetcat commented Jul 18, 2024

Alright, I think that's everything fixed that I can replicate locally:

  • Banner added
  • Bundles input required
  • Work check toggle
  • Task title array input rewritten
  • Preview disabled on last step

I've asked UX about what that help text should be. (Update: the tooltip has been removed.) Still not sure what's causing the display issues on your end; is there anything else you can share? What OS is this? Are the dependencies up-to-date?

@florkbr
Copy link
Contributor

florkbr commented Aug 8, 2024

@randomnetcat not blocking getting this in - but I noticed if I fill out the details on step 2 for any content type, then switch back and choose a different content type, the fields on the second page remain filled for the new type. I think we discussed this before - but it would be nice to have a way to "clear" the form fields. I know we have overlap between each type - but if I make a mistake or don't want to carry things over - it would be nice to "reset".

@florkbr
Copy link
Contributor

florkbr commented Aug 8, 2024

I did notice it is possible to submit a quickstart with task 1's title being empty (although it looks to be required with the red asterisk):
image

Which results in a weird UI:
image

Copy link
Contributor

@florkbr florkbr left a comment

Choose a reason for hiding this comment

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

@randomnetcat overall I think this is a great improvement over what we have in stage today! We can take over the remaining issues. Thank you for your work!

:shipit:

@florkbr florkbr merged commit 1a0b858 into master Aug 8, 2024
6 checks passed
@Hyperkid123 Hyperkid123 deleted the creator branch August 12, 2024 08:11
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.

3 participants