-
Notifications
You must be signed in to change notification settings - Fork 0
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
Feat/plus icon shortcut #22
base: main
Are you sure you want to change the base?
Conversation
…at/plus-icon-shortcut
</li> | ||
</ul> | ||
)} | ||
</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a suggestion not a pressing fix : When the dropdown is opened or closed, adding a small animation or transition to enhance the user experience.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I completely agree, I attempted to find something on tailwind, but was unable to get a transition to work.
This is the tailwind code I attempted, but was unable to get a result.
className={`bg-cyan-600 m-4 shadow-md rounded-md transition-all duration-300 ease-in-out transform ${ isDropDownOpen ? 'scale-100 opacity-100' : 'scale-95 opacity-0'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does tailwind offer an invisible and visible style where you can always render the elements but conditionally rendering the styles?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
try this - I see it having a very smooth transition in and out
<div className='dropDownPlus'>
<button className="m-auto cursor-pointer" onClick={toggleDropDown}>
<img
className="m-auto w-1/4 cursor-pointer"
src={isDropDownOpen ? plusOpen : plus}
alt="Teal Plus Icon"
/>
</button>
<ul
className={`bg-cyan-600 m-4 shadow-md rounded-md transition-all duration-700 ease-in-out transform ${
isDropDownOpen ? 'scale-100 opacity-100 visible' : 'scale-95 opacity-0 invisible'
}`}
>
<li className="p-2 hover:bg-gray-200 rounded text-center mb-2">
<Link to="/newContact">Create New Contact</Link>
</li>
<li className="p-2 hover:bg-gray-200 rounded text-center mb-2">
<Link to="/companies/new">Create New Company</Link>
</li>
<li className="p-2 hover:bg-gray-200 rounded text-center">
<Link to="/jobapplications/new">Create New Job Application</Link>
</li>
</ul>
</div>
```
cy.get('.bg-cyan-600 > :nth-child(3) > a').click() | ||
cy.url('match', 'http://localhost:3000/jobapplications/new'); | ||
}) | ||
}) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
idk if i'm missing this in the tests but you are testing for the presence of dropdown elements, may be worth testing the isDropDownOpen state directly just to check it's being toggled as expected. I might of missed it in your code. Correct me if i'm wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
||
**Chirchirillo, Joe** | ||
- [Github](https://github.com/jchirch) | ||
- [LinkedIn](https://www.linkedin.com/in/joechirchirillo/) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You had me at "HEY YOU!!!!!" Great addition to the readme and instructions to boot!
cypress/e2e/landingSpec.cy.js
Outdated
cy.get('.bg-cyan-600 > :nth-child(2) > a').click() | ||
cy.url('match', 'http://localhost:3000/companies/new'); | ||
}) | ||
it('should render dropdown menu with link to create new contact', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should say job application instead of "new contact"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, thanks. Fixed.
cy.get('.bg-cyan-600 > :nth-child(3) > a').click() | ||
cy.url('match', 'http://localhost:3000/jobapplications/new'); | ||
}) | ||
}) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
</li> | ||
</ul> | ||
)} | ||
</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
try this - I see it having a very smooth transition in and out
<div className='dropDownPlus'>
<button className="m-auto cursor-pointer" onClick={toggleDropDown}>
<img
className="m-auto w-1/4 cursor-pointer"
src={isDropDownOpen ? plusOpen : plus}
alt="Teal Plus Icon"
/>
</button>
<ul
className={`bg-cyan-600 m-4 shadow-md rounded-md transition-all duration-700 ease-in-out transform ${
isDropDownOpen ? 'scale-100 opacity-100 visible' : 'scale-95 opacity-0 invisible'
}`}
>
<li className="p-2 hover:bg-gray-200 rounded text-center mb-2">
<Link to="/newContact">Create New Contact</Link>
</li>
<li className="p-2 hover:bg-gray-200 rounded text-center mb-2">
<Link to="/companies/new">Create New Company</Link>
</li>
<li className="p-2 hover:bg-gray-200 rounded text-center">
<Link to="/jobapplications/new">Create New Job Application</Link>
</li>
</ul>
</div>
```
Type of Change
Description
This adds functionality to the plus sign on the menu bar and adds a dropdown menu with links to create new resources (Contacts, Companies, Job Applications).
Motivation and Context
By adding shortcuts to the plus icon, users will be able to swiftly create new resources for job hunting upon logging in. This reduces the need for users to have to navigate deeper in the app than necessary.
All styling was done to closely match the mockup, but without an official style guide, minor styling refactoring may be necessary in the future. Additionally, it allows developers in the immediate future the ability to build their "Create" pages.
Related Tickets
All tickets surrounding creating a new resource.
Screenshots (if appropriate):
Added Test?
Checklist: