feat: add event images #652
feat: add event images #652
Conversation
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.
@ahmadabdolsaheb this is technically my PR now, but what do think to the suggestions?
boxSize="100%" | ||
maxH="300px" | ||
src={data.event.image} | ||
alt="The freeCodeCamp logo" |
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.
Can we add the alt text when adding the image and then use it here?
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.
Sure a field called description should do 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.
Thinking about it, this is purely decorative. We can just require alt=""
since screenreaders can safely ignore it.
client/src/modules/dashboard/Events/components/EventFormUtils.ts
Outdated
Show resolved
Hide resolved
dcf9b5a
to
a062ab6
Compare
eb29466
to
42aa835
Compare
@@ -0,0 +1,13 @@ | |||
import { MigrationInterface, QueryRunner } from 'typeorm'; |
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.
Not sure, but since we are introducing "image_url" for first time here we would not need a migration file?
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.
Nope, we're not using explicit migration files now, since typeorm can set up the db for us without them. I'll remove this file while I sort out the conflicts.
Co-authored-by: Oliver Eyton-Williams <ojeytonwilliams@gmail.com>
It's a purely decorative image
42aa835
to
560c39e
Compare
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.
LGMT
Recreated after I accidentally merged the last one: #651