codeforpdx / PASS

PASS project - with HMIS module integration
MIT License
28 stars 25 forks source link

389 Revamped Home Page #405

Closed Raspber closed 11 months ago

Raspber commented 12 months ago

Revamped Home Page


This PR:

1. Added some basic images along with filler text so the home page is not empty. 2. Request demo button call to action.
3. Moved assets to public folder


The files this PR effects:

Components

Tests

No test files are affected

Other Files


Screenshots (if applicable):

Current green theme with revamped home page revamped-home-page


Future Steps/PRs Needed to Finish This Work (optional):


Issues needing discussion/feedback (optional):

Raspber commented 12 months ago

See feedback.

Will make the changes required👍🏽

andycwilliams commented 12 months ago

I personally like making purple the main color. However, in my meetings with the UX team, it's become clear that choosing a theme/color palette is actually much more complicated than we might have assumed. As part of our overall plan, finding a new theme is a lower priority. So in my opinion we can hold off on changing that for the time being.

Raspber commented 11 months ago

I personally like making purple the main color. However, in my meetings with the UX team, it's become clear that choosing a theme/color palette is actually much more complicated than we might have assumed. As part of our overall plan, finding a new theme is a lower priority. So in my opinion we can hold off on changing that for the time being.

I personally like making purple the main color. However, in my meetings with the UX team, it's become clear that choosing a theme/color palette is actually much more complicated than we might have assumed. As part of our overall plan, finding a new theme is a lower priority. So in my opinion we can hold off on changing that for the time being.

Deleted the purple per request.

xscottxbrownx commented 11 months ago

@Raspber - will you still have access to these graphics after merging into codebase?

Debating if we should add the purple images in the codebase or not, since we aren't using yet - and not sure if that's what team will land on.

Raspber commented 11 months ago

@Raspber - will you still have access to these graphics after merging into codebase?

Debating if we should add the purple images in the codebase or not, since we aren't using yet - and not sure if that's what team will land on.

Yes, i do have access to these images. I think I can get rid of the purple images because ux team is most likely going to come up with a brand new home page. Ka Hung just wanted something in there so it wasnt so plain. In less the ux team may want to use some of these images.

andycwilliams commented 11 months ago

@Raspber - will you still have access to these graphics after merging into codebase?

Debating if we should add the purple images in the codebase or not, since we aren't using yet - and not sure if that's what team will land on.

We can talk more about it at the meeting but I think we shouldn't include anything until we know for sure that we need it. And yeah it all depends on the UX team anyhow so I'd wait at least until we talk to them.

xscottxbrownx commented 11 months ago

@Raspber at the meetup we agreed, if you can delete the purple image files from this - we can approve and merge.

xscottxbrownx commented 11 months ago

We don't need the images in both public and src folders...

I did ask for help at last night's meeting to research what the deal is for vite and proper way to use images. So, hopefully we can get a "correct" answer and procedure for us.

ogorman89 commented 11 months ago

I did ask for help at last night's meeting to research what the deal is for vite and proper way to use images. So, hopefully we can get a "correct" answer and procedure for us.

@xscottxbrownx The images are loading fine from the public folder when I build locally. Was there something else I needed to do to reproduce the issue?

My understanding of your comment is that we just want to figure out the best practice for storing these assets going forward.

Edit: referenced incorrect folder.

xscottxbrownx commented 11 months ago

I did ask for help at last night's meeting to research what the deal is for vite and proper way to use images. So, hopefully we can get a "correct" answer and procedure for us.

@xscottxbrownx The images are loading fine from the public folder when I build locally. Was there something else I needed to do to reproduce the issue?

My understanding of your comment is that we just want to figure out the best practice for storing these assets going forward.

Edit: referenced incorrect folder.

Yes photos and actually the HomeSection components are loading fine now because we fixed the actual issue which was improper destructuring of props being passed to the component.

But now 2 things...

For this PR 1) photos have been put in both src and public folders, just needs to be one. Right now they are being loaded from src/assets as you can see in Home.jsx - but there was a past PR that Ka Hung ended up adding the public folder instead because although it loaded on Development, there were issues on Master.

In General 2) need to research what is proper way to use/store/access images and paths in Vite. So, we can move forward in future with no issues. My 5 second research was that public folder was one way they could be handled. Just need to figure it out, and then be consistent from there forward.

leekahung commented 11 months ago

Yeah, it seems like Vite doesn't pick up the source files for images under src/assets in the production build (see issue #406). Images end up breaking there even though they show up locally in the development build.

Having the image files stored in the public directory is one way to have static assets built with the site. (See link in issue #406 for Vite documentation and PR #407 on how I implement it here on PASS)

Since that's the case, I agree with Scott. It'll be good to have them all inside public. Though I wonder if we make folders inside public mainly for images like public/assets or public/images and place it in them instead of the root of public (have not tried it yet in the project).

andycwilliams commented 11 months ago

For what it's worth, the professional projects I've seen have had images in the public folder, so that seems to be best practice anyhow.

Also, having folders inside of public ought to work fine. Though I haven't experimented very much with it.

Otherwise, we're very close to merging this, no?

xscottxbrownx commented 11 months ago

Otherwise, we're very close to merging this, no?

Yes, we just need to:

  1. get rid of the assets from the src folder
  2. put in the public path for the image src's in the Home component instead of the source path.
xscottxbrownx commented 11 months ago

@Raspber why doing this here on line 43, but not on line 53 and 60?

Screenshot 2023-09-23 at 6 04 42 AM

The vite documentation here says it should be /assets/filename.fileextenstion (eventhough it seems to work either way.)


@leekahung do you know of anybody still currently working on the driver's license barcode data capture idea? This PR currently deletes the barcode demo from src/assets (probably because I wasn't clear enough in my number 2 above in my last comment), and not sure if we should delete that yet.

leekahung commented 11 months ago

@leekahung do you know of anybody still currently working on the driver's license barcode data capture idea? This PR currently deletes the barcode demo from src/assets (probably because I wasn't clear enough in my number 2 above in my last comment), and not sure if we should delete that yet.

At the moment, no. Think Brian and Jared were the people working on them before. With regards to the barcode demo file, it's simply there to help developers test out the feature initially since it's saved with the repo. But it's not really necessary, I think.

Raspber commented 11 months ago

@Raspber why doing this here on line 43, but not on line 53 and 60?

Screenshot 2023-09-23 at 6 04 42 AM

The vite documentation here says it should be /assets/filename.fileextenstion (eventhough it seems to work either way.)


@leekahung do you know of anybody still currently working on the driver's license barcode data capture idea? This PR currently deletes the barcode demo from src/assets (probably because I wasn't clear enough in my number 2 above in my last comment), and not sure if we should delete that yet.

Did not see that. Thank you. Will get it removed. I can also add the barcode image if youd like.

xscottxbrownx commented 11 months ago

@Raspber - all good. Fixed. Merging now.

Great work on all this!

Raspber commented 11 months ago

@Raspber - all good. Fixed. Merging now.

Great work on all this!

Thanks for looking out! Its appreciated!

leekahung commented 11 months ago

Could you update the PR details with the changes included @Raspber? Thanks!