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

new pr #105

Closed
wants to merge 5 commits into from
Closed

new pr #105

wants to merge 5 commits into from

Conversation

RandolphTang
Copy link
Collaborator

Author:

What changes were made?

All officer info is updated

Why are these changes important/necessary?

(If applicable) Screenshots of your changes. Providing an “old vs. new” comparison would be great!

Copy link

github-actions bot commented Sep 28, 2024

Visit the preview URL for this PR (updated for commit b0543f2):

https://swecc-org--pr105-randolph-0h72fp00.web.app

(expires Mon, 14 Oct 2024 01:28:10 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: d5f37faadb442ba8fa7c35b669f11be1475c4891

<p>
Be part of a community of over 1000 people pursuing a career in
software engineering. <br />
Gain access to resources, including resume reviews, mock interviews,
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't put breaks; use built in wrapping

"email": "[email protected]"
},
{
"name": "Bhavya Linga",
Copy link
Contributor

Choose a reason for hiding this comment

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

Bhavya no longer with us

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think Deloitte is sponsoring us yet :(

<div className="WhySponsor">
<h2>Why Sponsor</h2>
<p id="HowToSponsorIntro">
Sponsoring SWECC to connect with 1,000+ UW CS students. Our <br />
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't use breaks, and change copy to

"Sponsoring the Software Engineering Career Club gives you access to over 1000 students, >70% of which are part of the Allen School, and >90% in computing related majors. Partner with us to gain access to our resume book, recruiting/promotional events, talent pool, and more.

<div className="HowToSponsor">
<h2>How to Sponsor</h2>
<p>
We appreciate all forms of your supports, you can support us by:
Copy link
Contributor

Choose a reason for hiding this comment

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

Others have partnered with us by...

  • Hosting informational/promotional panels
  • Sponsoring our events, such as hackathons and programming competitions
  • Getting involved in our professional mentorship program

And much more!

</div>

<div className="contact-info">
<h2 id="contact-info-title">Connect with Us</h2>
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought we weren't using this? We can set it up with SendGrid now if we want, although idk about preventing the key from getting exposed on here

<div className="posts" ref={scrollContainerRef}>
<InstagramEmbed
className="instaPost slide-up"
url="https://www.instagram.com/p/C7uRAftvEKV/?utm_source=ig_web_copy_link"
Copy link
Contributor

Choose a reason for hiding this comment

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

can you add the new post? or did you already?

Copy link
Member

Choose a reason for hiding this comment

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

Why are we hardcoding these urls?

Copy link
Member

@ShawnCollinge ShawnCollinge left a comment

Choose a reason for hiding this comment

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

Why do we have header.js and nav-bar.js with the same links? I didn't dive too deep into it but something to maybe look into for refactoring.

@@ -18,6 +19,7 @@
"react-scripts": "5.0.1",
"react-social-media-embed": "^2.3.5",
"react-transition-group": "^4.4.5",
"resend": "^4.0.0",
Copy link
Member

Choose a reason for hiding this comment

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

is this being used anywhere?

<ExternalRedirect to="https://mailman11.u.washington.edu/mailman/listinfo/sweccmailinglist" />
}
/>
<Route path="/join-Now" element={<JoinNow />} />
Copy link
Member

Choose a reason for hiding this comment

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

should probably lowercase the N to follow the rest.

)}
</div>
</div>
));

const postId = ["C2-o7roPbfN", "C23PyAWPYdW", "C2boAR4LeRy", "CzVNKp6r9hn", "CzVLbJWLwsI", "Cy4WTIJvVvB", "CyzVzMTJlhQ", "CyRondZPVVS", "CyCms3mLOLb", "Cxum20yOlkQ", "CxrxWcQLpSb"];
const postId = [
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason for hardcoding postIds? Are we not just wanting the most recent n posts? Maybe you can't answer this but maybe @elimelt can?

<NavLink to="/" className="nav-link">
HOME
</NavLink>
<NavLink to="/Events" className="nav-link">
Copy link
Member

Choose a reason for hiding this comment

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

Does this link work? You lowercased it in the routes.

<NavLink to="/Events" className="nav-link">
EVENTS
</NavLink>
<NavLink to="/About" className="nav-link">
Copy link
Member

Choose a reason for hiding this comment

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

ditto here

<div className="posts" ref={scrollContainerRef}>
<InstagramEmbed
className="instaPost slide-up"
url="https://www.instagram.com/p/C7uRAftvEKV/?utm_source=ig_web_copy_link"
Copy link
Member

Choose a reason for hiding this comment

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

Why are we hardcoding these urls?

<NavItem
icon={<FcConferenceCall />}
route={"/About"}
route={"/Officers"}
Copy link
Member

Choose a reason for hiding this comment

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

This should also be lowercase.

expand={expand}
closeExpand={closeExpand}
tooltip="Officers"
label="Officers"
/>
<NavItem
icon={<FaHeart />}
route={"/Sponsor"}
Copy link
Member

Choose a reason for hiding this comment

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

lowercase?

@@ -221,7 +195,7 @@ function DropdownMenu(props) {
<NavLink to="/" onClick={props.closeExpand}>
<DropdownItem leftIcon={<FcHome />}>Home</DropdownItem>
</NavLink>
<NavLink to="/About" onClick={props.closeExpand}>
<NavLink to="/Officers" onClick={props.closeExpand}>
Copy link
Member

Choose a reason for hiding this comment

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

lowercase?

<nav className="officer-sidebar">
<h2>Year</h2>
<ul>
{[2024, 2023, 2022, 2021].map((year) => (
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can not hardcode years, can check if after september then add the next year? Starting with 2021.


export default NotionMagicLinkEmail;

const main = {
Copy link
Member

Choose a reason for hiding this comment

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

Why don't we move this css into it's own spot and I think the css can be shared across this and plaid-verify?

function intPostsWidget() {
return (
<BeholdWidget
onLoad={() => console.log("Loaded!")}
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't have console.logs in prod please.

@ShawnCollinge ShawnCollinge deleted the Randolph branch November 24, 2024 17:14
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.

3 participants