-
Notifications
You must be signed in to change notification settings - Fork 3
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
731 saved comapnies updates UI #735
Conversation
useEffect(() => { | ||
setSavedIsUpdated(data.saved_is_updated); | ||
}, [data.saved_is_updated]); |
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.
nit: format it
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.
Fixed it
></StarForLike> | ||
<div className={css['bell-container']}> | ||
<BellForUpdates | ||
className={savedIsUpdated ? '' : 'hidden'} |
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.
Do we need it? We do not use className
prop in the BellForUpdates
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.
Deleted redundant code
<div className={css['bell-container']}> | ||
<BellForUpdates | ||
className={savedIsUpdated ? '' : 'hidden'} | ||
savedIsUpdated={savedIsUpdated} |
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'm wondering why we can't use data.saved_is_updated
directly.
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.
Fixed it
@@ -107,7 +107,7 @@ | |||
} | |||
|
|||
.content__common-info { | |||
width: 1169px; | |||
width: 1150px; |
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.
Have you tested it with screens different than 1150?
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.
Everything seems to work ok with this task on various resolutions, but this whole issue with hardcoded sizes in pixels will be fixed in future
|
||
const { Paragraph } = Typography; | ||
|
||
export default function ProfileCard({ isAuthorized, data }) { | ||
const { user } = useAuth(); | ||
const [isSaved, setIsSaved] = useState(data.is_saved); | ||
const [savedIsUpdated, setSavedIsUpdated] = useState(data.saved_is_updated); |
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'm wondering whether we can pass a callback to update it instead of cleaning it in the state.
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.
Please check the new implementation, whether it is correct, or not
created visuall representation for saved companies updates, whenever saved company is updated, user will see a bell on its profile card
#731