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

[90] link refactor - terrianx/link refactor #140

Merged
merged 9 commits into from
Dec 29, 2023
Merged

Conversation

terrianx
Copy link
Contributor

updated all links including gatsby links, anchors, buttonlinks (primary and secondary)

links now all look like

<Link href="some external site" prop1=x prop2=y>Click here</Link>
or
<Link to="some internal page" prop1=x prop2=y>Click here</Link>

href must be defined for an external link
to must be defined for internal page, otherwise it redirects to home page
any props can be passed down all the way to the original <a> or <GatsbyLink> element defined in components/Link.js
children can also be rendered in Link

i tested every link after making this change. every link that is currently working still works. #127 and #131 are some currently broken links.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

connectList still needs to be updated. some pages are not included in this mvp im pretty sure.

need to fill in interest form

also wondering if the footer link at the very bottom for "HARVEST MISSION COMMUNITY CHURCH" should redirect to this new site or the old one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

clicking ann arbor on location no longer does nothing. it opens a new tab at the home page.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i know this page is on hold right now but i still changed it since we are probably going to use it in the near future?

wondering if the link to the lg form (check out a lifegroup) should link directly to the form itself or the internal page (which is what it currently does)

@@ -181,12 +173,12 @@ const Footer = () => {
<h2 className="text-Shades-0 text-sm md:text-lg font-normal mb-0 ml-1 tracking-medium-wide md:tracking-normal">
2023
</h2>
<a
<Link
href="https://annarbor.hmcc.net"
Copy link
Collaborator

Choose a reason for hiding this comment

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

should this redirect to current gatsby site?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

im not sure, i think it probably should. ill update it to point to the new site

@@ -133,16 +133,14 @@ const Header = () => {
{locationsList.map((item, index) => (
<Link
key={`browseLink-${index}`}
to={item.route}
target="_blank"
href={item.route}
Copy link
Collaborator

Choose a reason for hiding this comment

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

for location list, need to use to instead 'href' you can do ternary opration for the which Link and properties to use:
Here is my suggestion

index === 0 ? (<Link key={`browseLink-${index}`} to={item.route}  className={`${textStyle} hover:bg-[#0C2966] py-2 px-4 border-b-[0.1px] border-gray-100 tracking-[0.96px]`}>{item.title}</Link>) : (<Link key={`browseLink-${index}`} href={item.route}  className={`${textStyle} hover:bg-[#0C2966] py-2 px-4 border-b-[0.1px] border-gray-100 tracking-[0.96px]`}>{item.title}</Link>)

i think this works. i do not know full syntax for reactjs for this ternary operation. but hopefully it works.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok ive fixed this

terrianx and others added 2 commits December 28, 2023 20:49
Co-authored-by: thedomcab <9808478+thedomcab@users.noreply.github.com>
@terrianx terrianx merged commit 56e4c20 into main Dec 29, 2023
4 checks passed
@terrianx terrianx deleted the terrianx/link-refactor branch December 29, 2023 20:49
@hmccaa hmccaa linked an issue Feb 5, 2024 that may be closed by this pull request
xumaple pushed a commit that referenced this pull request Jul 16, 2024
* Link refactor skeleton

* Added conditional rendering for links

* Small whitespace fix

* Usage comment

* Updated button links to use custom link

* Refactored all links

* Removed debug

* Change to redirection to home fix

Co-authored-by: thedomcab <9808478+thedomcab@users.noreply.github.com>

* Fixed to redirection and ann arbor location link

---------

Co-authored-by: thedomcab <9808478+thedomcab@users.noreply.github.com>
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