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

Add List Feature #31

Closed
wants to merge 6 commits into from
Closed

Add List Feature #31

wants to merge 6 commits into from

Conversation

ka-lange
Copy link
Contributor

@ka-lange ka-lange commented Nov 9, 2023

Description

Added new Favicon with logo and list view functionality. In order for the list button in the action menu to change the state of the client card, the state is created on the home page. It is then passed to the action menu and handled by the list view button. Once the view state is changed, it is passed into the Client Card component and used in a card conditional for view.

Notes

  • Could not see favicon because of the testing environment, but docs state that the Next app should render it in the head automatically
  • new global css classes of the card-view and list-view were created so the list items aren't always in a grid like the card view

Future Issues

  • The styling can be improved on the list items, but I wanted to get it functional first
  • A big future feature could be saving the view style and filters, sort, etc in the session/local storage so selected preferences will save on page load like theme preference does

Closes Issue #13

@ka-lange ka-lange requested a review from devlarabar as a code owner November 9, 2023 22:03
Copy link

vercel bot commented Nov 9, 2023

Someone is attempting to deploy a commit to a Personal Account owned by @devlarabar on Vercel.

@devlarabar first needs to authorize it.

devlarabar
devlarabar previously approved these changes Nov 10, 2023
Copy link
Owner

@devlarabar devlarabar left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks, Karissa.

I noticed that the client emails disappear after selecting and then deselecting the list view; this should be fixed before merging
My other concern is that the list view looks a little wonky on smaller devices (see below).

Screenshot 2023-11-10 140508
Screenshot 2023-11-10 140454

The responsiveness can be fixed in a future issue, though. Let me know if you'd like to work on that, otherwise I'm happy to take it on myself since I know you'll be pretty busy starting next week!

@devlarabar devlarabar self-requested a review November 10, 2023 19:12
@devlarabar devlarabar dismissed their stale review November 10, 2023 19:13

Overlooked client email bug

@AhmedHHamdy AhmedHHamdy mentioned this pull request Dec 5, 2023
2 tasks
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.

2 participants