feat: show project preview #43967

ojeytonwilliams · 2021-10-22T09:35:26Z

Shows a modal which previews the solution to the final step of a practice project.

Not all practice projects have solutions, yet, and so the modal doesn't appear for registration-form. Once they all do we should lock that down with a test.

@ahmadabdolsaheb if you've got a chance, could you look at smartening up the Modal? It... works, but it's not pretty.

@raisedadead since this is an upcoming feature, I disabled the tests with SHOW_UPCOMING_CHANGES so we can run them locally when we want and then in CI when we're ready. Does that seem reasonable?

gitpod-io · 2021-10-22T09:35:29Z

ghost · 2021-10-22T09:37:16Z

👀 Review this PR in a CodeSee Review Map

Preview the Review Map

Legend

ahmaxed · 2021-10-25T13:56:33Z

So there are a couple of challenges with displaying the projects inside a modal.

projects have different ratios. if we decide to show the whole finished project, the size of the modals will vary.
we need to add the logic for that flash animation after the preview modal has been closed to the users can see.

raisedadead · 2021-10-25T14:15:33Z

Does that seem reasonable?

Yes - absolutely.

ojeytonwilliams · 2021-10-25T14:23:01Z

projects have different ratios. if we decide to show the whole finished project, the size of the modals will vary.

Aren't they all responsive? They have to be shown in the preview pane and that's something that the user controls.

we need to add the logic for that flash animation after the preview modal has been closed to the users can see.

Do you mean the 'Noticable Editor' animation? Yep, I'll track that in the issue.

moT01 · 2021-11-18T20:07:49Z

I rebased this, fixed the conflicts, and made the modal bigger - I hope you don't mind @ojeytonwilliams. I'm not sure if we want to try and get it in like this or add that animation @ahmadabdolsaheb mentioned to focus users attention. Is there any part of that done? Mocks or anything? I could try and come up with something if needed. I vote to do it on another PR, but either way works.

ojeytonwilliams · 2021-11-18T20:37:01Z

Not at all, thanks Tom. I vote for handling those other features in another PR, too.

ahmaxed · 2021-11-19T14:06:59Z

@moT01, nice touch with adjusting the height. I is very likely that users will initially develop the projects with the preview in a horizontal frame. On the other hand, I think we should keep the width to the standard modal with that we use elsewhere; otherwise the buttons are going to get stretched too much.

Here is a preview:

Notice the white line on the bottom of the preview:

Here is Tom's downtown :)
Notice the lines to the left and bottom of the preview.

One of the lines is associated with the margin we set in builders
<body id='display-body'style='margin:8px;'>

I have not figured out the bottom one :)

Finally, a loader might be a nice touch. But that could wait.

ahmaxed · 2021-11-19T14:11:31Z

what do you think of a no padding modal for the preview?

moT01 · 2021-11-19T14:27:43Z

I'll look into those margins.

I is very likely that users will initially develop the projects with the preview in a horizontal frame

I'm not sure what you mean by this.

what do you think of a no padding modal for the preview?

I'm in favor of that - I'll see what it looks like on a project with scroll bars.

I think we should keep the width to the standard modal with that we use elsewhere

I'm not against this - but we're trying to display a whole web page, some extra room might be nice.

…projects

moT01 · 2021-11-20T02:51:44Z

I did some things to fix up @ahmadabdolsaheb's suggestions. One of the issues was due to an 8px margin we add to all the previews. I added a second template, specific for the demo previews, that doesn't include the margin (see my last commit). I think we could use a little discussion on it.

Some options:

Do what I added here - use the template with the 8px margin when viewing the preview of the editor code (challenges), and the template without the margin for the demo preview (modal)
Use the template without the margin for everything
Use the template with the margin for everything

Some pros and cons:

Option 1:
👍 The editor preview behaves more like a real browser - which, I'm guessing, is why it was added in the first place?
👍 Doesn't affect existing curriculum/practice projects
👍 Demo modal looks good
👎 Uses two different templates. A web page will likely look different when in the editor vs the demo. I assume we will have a modal like this on the cert page to display user's cert solutions, as well - something to keep in mind

Option 2:
👍 Consistent across the different previews
👍 Demo modal looks good
👎 It made a number of the practice projects and existing lessons look a little worse, since some text and other things hug the side of the preview
👎 Potentially breaks tests or instructions from making sense? It probably wouldn't be many if it does.

Option 3:
👍 Consistent across the different previews
👍 Doesn't affect existing curriculum/practice projects
👍 The editor preview behaves more like a real browser
👎 The demo modal looks rough

I would appreciate your thoughts on this @nhcarrigan & @ShaunSHamilton

Side note - I believe these projects are missing solutions on the last step so the modal doesn't show up:
Ferris wheel
Grid Magazine
Photo Gallery
Registration Form
Nutrition Label

Probably something for another PR.

naomi-lgbt · 2021-11-22T16:37:02Z

I assume we will have a modal like this on the cert page to display user's cert solutions, as well - something to keep in mind

Honestly, if we could rig it up so that their project opens in a new tab I think that would be ideal.

Side note - I believe these projects are missing solutions on the last step so the modal doesn't show up:

I will open a PR to fix this today. 🙂

I think going with option 1 is the best - two templates is not ideal, but if it produces the best end result then 🤷🏻

ShaunSHamilton · 2021-11-22T17:01:19Z

As for the margin, I believe this falls under the same discussion about whether we keep using normalize.css or not. I was largely for removing it, because it alters the page such that it could look different on a Camper's local development, and we have at least two practice projects going over the common use of:

* {
  margin: 0;
  padding: 0;
}

Which normalize.css defeats the purpose of.

My Current Thinking

Remove anything that makes the layout look different to a Camper developing locally. If this includes the 8px margin, then so be it.

I believe these projects are missing solutions on the last step so the modal doesn't show up:

I will open a PR to fix this today. 🙂

@nhcarrigan There is a PR, we should handle first: #43971 - same idea

ojeytonwilliams · 2021-11-22T18:11:27Z

I was largely for removing it, because it alters the page such that it could look different on a Camper's local development

If challenges/projects look slightly different on different browsers, that's fine, I think. We want it to feel as real as possible.

Remove anything that makes the layout look different to a Camper developing locally. If this includes the 8px margin, then so be it.

That's my thinking, too. That's what I did in #42195

…inished projects" This reverts commit 0db11a0.

ojeytonwilliams

I can't review my own PRs, but I'm happy with Tom's changes.

naomi-lgbt · 2021-11-24T19:53:44Z

~~I'm still seeing the margins on the preview.~~

Nevermind - helps if I do a git pull. 🤦🏻

naomi-lgbt

This looks good to me.

ShaunSHamilton

I cannot say I fully followed the framer.js logic, but this appears to be working.

Otherwise, I worry about my personal confusion between the preview and the practice project preview - might be something I get over.

client/i18n/locales/english/translations.json

client/src/templates/Challenges/components/project-preview-modal.tsx

Co-authored-by: Shaun Hamilton <shauhami020@gmail.com>

ShaunSHamilton

LGTM 👍

ahmaxed · 2021-11-29T09:06:11Z

This is LGMT if we are fine with removing the style='margin:8px;'>

naomi-lgbt

I'm happy with this.

ojeytonwilliams added 9 commits November 18, 2021 11:52

feat: add data for preview to challengeMeta

e04ac51

feat: allow creation of project preview frames

8deef30

feat: make project preview data available for frame

0807a58

refactor: simplify reducer

cebda66

feat: show project preview for first challenge

943aa97

feat: show project preview on MultiFile challenges

f5c1343

test: check for presence/absence of preview modal

394d2f7

fix: simplify previewProject saga

56f2193

test: uncomment project preview test

d56e016

moT01 force-pushed the feat/show-project-preview branch from 02b2ee3 to d56e016 Compare November 18, 2021 18:35

fix: increase modal size + change modal title

e3ea689

ojeytonwilliams marked this pull request as ready for review November 18, 2021 20:37

ojeytonwilliams requested a review from a team as a code owner November 18, 2021 20:37

moT01 closed this Nov 18, 2021

moT01 reopened this Nov 18, 2021

moT01 mentioned this pull request Nov 18, 2021

Project-Based Curriculum Features #43668

Closed

ahmaxed added 2 commits November 19, 2021 16:34

modal-footer

fda650b

feat: adjust preview size

fbcd160

fix: remove margin, padding, and line-height for preview of finished …

0db11a0

…projects

gikf added the status: waiting review To be applied to PR's that are ready for QA, especially when additional review is pending. label Nov 20, 2021

moT01 added 2 commits November 24, 2021 11:24

Revert "fix: remove margin, padding, and line-height for preview of f…

b95e10a

…inished projects" This reverts commit 0db11a0.

fix: remove margin on all previews

9a11bc4

ojeytonwilliams commented Nov 24, 2021

View reviewed changes

naomi-lgbt previously approved these changes Nov 24, 2021

View reviewed changes

ShaunSHamilton requested changes Nov 24, 2021

View reviewed changes

ShaunSHamilton reviewed Nov 24, 2021

View reviewed changes

client/src/templates/Challenges/components/project-preview-modal.tsx Outdated Show resolved Hide resolved

refactor: use closeModal('projectPreview') for clarity

2479205

Co-authored-by: Shaun Hamilton <shauhami020@gmail.com>

ojeytonwilliams dismissed naomi-lgbt’s stale review via 2479205 November 25, 2021 07:18

ojeytonwilliams added 2 commits November 25, 2021 08:27

fix: get started -> start coding!

7ce58f4

fix: update closeModal type

d00fd22

ShaunSHamilton approved these changes Nov 25, 2021

View reviewed changes

ojeytonwilliams mentioned this pull request Nov 26, 2021

fix: hide file tabs on legacy challenges #44291

Merged

ahmaxed approved these changes Nov 29, 2021

View reviewed changes

naomi-lgbt approved these changes Nov 29, 2021

View reviewed changes

ShaunSHamilton merged commit bb7893d into freeCodeCamp:main Nov 29, 2021

ojeytonwilliams deleted the feat/show-project-preview branch November 29, 2021 18:43

lem0n4id mentioned this pull request Mar 22, 2022

Show a message after completing a project - RWD beta #45501

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: show project preview #43967

feat: show project preview #43967

ojeytonwilliams commented Oct 22, 2021

gitpod-io bot commented Oct 22, 2021

ghost commented Oct 22, 2021 •

edited by ghost

Legend

ahmaxed commented Oct 25, 2021

raisedadead commented Oct 25, 2021

ojeytonwilliams commented Oct 25, 2021

moT01 commented Nov 18, 2021

ojeytonwilliams commented Nov 18, 2021

ahmaxed commented Nov 19, 2021 •

edited

ahmaxed commented Nov 19, 2021

moT01 commented Nov 19, 2021 •

edited

moT01 commented Nov 20, 2021

naomi-lgbt commented Nov 22, 2021

ShaunSHamilton commented Nov 22, 2021

ojeytonwilliams commented Nov 22, 2021

ojeytonwilliams left a comment

naomi-lgbt commented Nov 24, 2021 •

edited

naomi-lgbt left a comment

ShaunSHamilton left a comment

ShaunSHamilton left a comment

ahmaxed commented Nov 29, 2021

naomi-lgbt left a comment

feat: show project preview #43967

feat: show project preview #43967

Conversation

ojeytonwilliams commented Oct 22, 2021

gitpod-io bot commented Oct 22, 2021

ghost commented Oct 22, 2021 • edited by ghost

Legend

ahmaxed commented Oct 25, 2021

raisedadead commented Oct 25, 2021

ojeytonwilliams commented Oct 25, 2021

moT01 commented Nov 18, 2021

ojeytonwilliams commented Nov 18, 2021

ahmaxed commented Nov 19, 2021 • edited

ahmaxed commented Nov 19, 2021

moT01 commented Nov 19, 2021 • edited

moT01 commented Nov 20, 2021

naomi-lgbt commented Nov 22, 2021

ShaunSHamilton commented Nov 22, 2021

ojeytonwilliams commented Nov 22, 2021

ojeytonwilliams left a comment

Choose a reason for hiding this comment

naomi-lgbt commented Nov 24, 2021 • edited

naomi-lgbt left a comment

Choose a reason for hiding this comment

ShaunSHamilton left a comment

Choose a reason for hiding this comment

ShaunSHamilton left a comment

Choose a reason for hiding this comment

ahmaxed commented Nov 29, 2021

naomi-lgbt left a comment

Choose a reason for hiding this comment

ghost commented Oct 22, 2021 •

edited by ghost

ahmaxed commented Nov 19, 2021 •

edited

moT01 commented Nov 19, 2021 •

edited

naomi-lgbt commented Nov 24, 2021 •

edited