-
Notifications
You must be signed in to change notification settings - Fork 1
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] Styling Status Filter #73
Conversation
* [refactor] adjusting styling * styling fixes
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.
Made comments on the changes you should make. Styling looks really good. I just suggested these changes to make it easier for us later on if the designer decides to adjust margins / makes changes to the design.
components/StatusDropdown/styles.ts
Outdated
background: ${COLORS.white}; | ||
width: 10.5rem; | ||
border-radius: 0.5rem; | ||
padding-bottom: 0.5rem; |
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.
delete padding-bottom
border-radius: 0.5rem; | ||
padding-bottom: 0.5rem; | ||
`; | ||
|
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.
Make a new styled div. This is so we can house all the content in one place and add the padding around it (so that if Josh ever changes it again it's easy to change the padding directly from the figma). Use the styled div to wrap around the content (check my comments in index.tsx)
export const FilterContentDiv = styled.div`
display: flex;
flex-direction: column;
padding: 0.625rem 1.25rem 0.9rem 1.25rem;
align-content: space-between;
`;
`; | ||
|
||
export const CheckboxStyles = styled.input` | ||
cursor: pointer; |
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.
add margin-left: 0rem;
for some reason the button is not directly aligned with the apply button which we want
components/StatusDropdown/styles.ts
Outdated
align-items: center; | ||
cursor: ${({ isActive }) => (isActive ? 'pointer' : 'not-allowed')}; | ||
border: none; | ||
margin: 1.4rem auto 0.5rem auto; |
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.
auto not 0.5rem (we don't need this margin since we have a margin for the div wrapped around all the content
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.
It should look like margin: 1.4rem auto auto auto;
components/StatusDropdown/styles.ts
Outdated
background: ${COLORS.white}; | ||
border: none; | ||
border-radius: 6.25rem; | ||
display: flex; |
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.
move display: flex;
to the top so it looks more standard
components/StatusDropdown/styles.ts
Outdated
display: flex; | ||
align-items: center; | ||
gap: 0.5rem; | ||
margin-left: 1rem; |
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.
delete
components/StatusDropdown/styles.ts
Outdated
align-items: center; | ||
gap: 0.5rem; | ||
margin-left: 1rem; | ||
padding-top: 1rem; |
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.
delete
components/StatusDropdown/styles.ts
Outdated
align-items: center; | ||
gap: 0.5rem; | ||
margin-bottom: -1.5rem; | ||
margin-left: 1rem; |
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.
remove
const isApplyButtonActive = selectedStatus.length > 0; | ||
|
||
return ( | ||
<FilterDropdownStyles> |
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.
below this, add to wrap around all the filter content
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.
components/StatusDropdown/index.tsx
Outdated
</div> | ||
<ApplyButtonStyles isActive={isApplyButtonActive}> | ||
<ApplyFiltersText>APPLY</ApplyFiltersText> | ||
</ApplyButtonStyles> |
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.
below this, add
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.
great job you can squash and merge
What's new in this PR
Description
texts.ts
Screenshots
How to review
StatusDropdown folder, styles folder, schema, Filter folder (index.tsx), FilterBar folder,
Next steps
Relevant links
Online sources
Related PRs
CC: @itsliterallymonique