Skip to content
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: add event organizer logo form field and update event page #58

Merged

Conversation

r3yc0n1c
Copy link
Contributor

resolves #52

Summary:

  • added a new input field labeled Event Organizer logo (named logo-url) inside the EventBasicCreate component
  • updated the respective state change to show it on the EventShow Page

Screenshots:

Original Updated
ss-original ss-updated
Original Updated
ss-original ss-updated

@CLAassistant
Copy link

CLAassistant commented Nov 27, 2022

CLA assistant check
All committers have signed the CLA.

<Form.Control
name="logo-url"
type="url"
placeholder=""
Copy link
Member

Choose a reason for hiding this comment

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

Please add a placeholder with some link value. It will help the user know the correct format of data to be added to the form.

Thank you!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done! Please review the changes.
Thank You!

Comment on lines 60 to 62
<Image src={event.data.attributes["logo-url"]} width={100} />
</div>
<center><h6> Organizer </h6></center>
Copy link
Member

Choose a reason for hiding this comment

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

While you are here. Can you try to make the logo resize responsively?

@r3yc0n1c r3yc0n1c force-pushed the wip-52-add-event-organizer-logo-form-field branch from 34c9c6c to 1626248 Compare November 27, 2022 19:02
@@ -59,6 +59,7 @@ export const EventShow = ({ event, error, speaker, prsession }) => {
<div className={styles.event_logo}>
<Image src={event.data.attributes["logo-url"]} width={100} />
</div>
<center><h6> Organizer </h6></center>
Copy link
Member

Choose a reason for hiding this comment

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

@r3yc0n1c please use div with style text-align=center in place of center, please avoid using center since it is already depreciated.

Thank you!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done! let me know if I need to change something.
Thanks!

@r3yc0n1c r3yc0n1c force-pushed the wip-52-add-event-organizer-logo-form-field branch from 1626248 to 1fd0265 Compare November 28, 2022 18:05
@@ -59,6 +59,7 @@ export const EventShow = ({ event, error, speaker, prsession }) => {
<div className={styles.event_logo}>
<Image src={event.data.attributes["logo-url"]} width={100} />
</div>
<div style={{textAlign: 'center'}}><h6> Organizer </h6></div>
Copy link
Member

@Dnouv Dnouv Nov 29, 2022

Choose a reason for hiding this comment

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

@r3yc0n1c Please use className for styling since if we use inline styling, the customization of the components' style will be affected.

Because for any style changes needed, the user would have to do modifications in the component (EventBasicDetail.js) in case of inline styling; However, for separate className in a .css file, they can modify a single CSS class property in the style.css which is pretty convenient.

Thank you!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

got it! please check the changes now...
Thank you!

@r3yc0n1c r3yc0n1c force-pushed the wip-52-add-event-organizer-logo-form-field branch from 1fd0265 to a65c976 Compare November 29, 2022 06:18
@Dnouv
Copy link
Member

Dnouv commented Nov 29, 2022

LGTM! @r3yc0n1c Thank you for the hard work. Ready-to-merge.

cc: @Sing-Li

@Sing-Li Sing-Li merged commit 26279d2 into RocketChat:main Dec 2, 2022
Dnouv pushed a commit that referenced this pull request Sep 10, 2024
…form-field

feat: add event organizer logo form field and update event page
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.

[NEW] Add a new form field
4 participants