Skip to content

add ability to upload banner image for community page#199

Merged
mannynotfound merged 5 commits intomainfrom
manny/cas-238
Jul 22, 2022
Merged

add ability to upload banner image for community page#199
mannynotfound merged 5 commits intomainfrom
manny/cas-238

Conversation

@mannynotfound
Copy link
Copy Markdown
Contributor

  • adds a dashed box underneath the avatar upload/edit for a user to add a "banner image" to the community
  • banner image is displayed as a full cover css background image on the community masthead

open questions

  • do we want to limit banner file size specifically? this PR sets the limit at 5MB similar to proposal images
  • do we want to resize banners? currently this skips resizing banners and only applies resize logic to avatars

getRootProps: getBannerRootProps,
getInputProps: getBannerInputProps,
} = useDropzone({
onDrop: onDrop('community_banner', 'banner', MAX_FILE_SIZE, 1200),
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

added a 1200px resize on banners on david's suggestion

Copy link
Copy Markdown
Contributor

@germanurrus germanurrus left a comment

Choose a reason for hiding this comment

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

Looks good! made a couple of comments

@@ -179,16 +194,9 @@ export default function StepOne({
position: 'relative',
...(!logo ? { border: '1px dashed #757575' } : undefined),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same here border: '2px dashed #757575'

>
{banner ? (
<div
className="is-flex flex-1 is-flex-direction-column is-align-items-center is-justify-content-center"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same here adding rounded-lg

<div className="columns">
<div className="column is-12">
<div
className="is-flex is-flex-direction-column is-align-items-center is-justify-content-center cursor-pointer rounded-lg border-dashed-dark"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same here:
className={is-flex is-flex-direction-column is-align-items-center is-justify-content-center cursor-pointer rounded-lg ${!banner ? 'border-dashed-dark': ''}}

backgroundPosition: 'center',
}}
>
<div className={headerContainerClassNames}>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this might be more a design question but depending on the background image tabs will kind of look faded away:
Screen Shot 2022-07-21 at 10 52 50

@@ -143,13 +172,13 @@ function CommunityEditorProfile({
? { border: '1px dashed #757575' }
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this one should be border: '2px dashed #757575' I know it's not related to this PR but just to keep borders consistency

)}
{bannerImage?.imageUrl && (
<div
className="is-flex flex-1 is-flex-direction-column is-align-items-center is-justify-content-center"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can you add rounded-lg here too so image is rounded like the container?

<div className="columns">
<div className="column is-12">
<div
className="is-flex is-flex-direction-column is-align-items-center is-justify-content-center cursor-pointer rounded-lg border-dashed-dark"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

to keep it the same as the avatar, when the image is added I think we should remove the border, maybe something like this?
className={is-flex is-flex-direction-column is-align-items-center is-justify-content-center cursor-pointer rounded-lg ${!bannerImage ? 'border-dashed-dark': ''}}

Screen Shot 2022-07-21 at 11 12 34

}}
/>
)}
{!(isUpdating && image.file) ? (
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not sure about this change, I know it's kind of tricky but I didn't want to show the message 'Uploading' on top of the image when the user was just updating the community name and not uploading a new image, since the variable isUpdating comes form the hook I didn't have a better way to check what kind of update was done. If we change this, then uploading will show up for any text update on the community. Also if you didn't add any new image and want to keep it with the bloquies avatar it will still show "Uploading..."
Screen Shot 2022-07-21 at 11 31 50

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

its not the prettiest but ill add isUpdatingImage and isUpdatingBanner to distinguish this logic

Copy link
Copy Markdown
Contributor

@germanurrus germanurrus left a comment

Choose a reason for hiding this comment

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

Looks good! thanks @mannynotfound for the updates!

@mannynotfound mannynotfound merged commit 1c3fcf0 into main Jul 22, 2022
@mannynotfound mannynotfound deleted the manny/cas-238 branch July 22, 2022 16:11
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