Skip to content

Make event location links clickable #641

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.

Sign up for GitHub

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

Open
dan-foley wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Make event location links clickable #641

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 68 additions & 2 deletions client/src/features/modal/EventModal.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,60 @@ import dataService from "../../services/dataService";
import { useAuthContext } from "../../contexts/AuthContext";
import { useEventsContext } from "../../contexts/EventsContext";

// URLRegex matches strings that look like URLs, including:
// 1. URLs starting with "http://" or "https://" (e.g., "https://example.com/path")
// 2. URLs starting with "www." (e.g., "www.example.com")
// 3. Bare domain names without protocol or www (e.g., "example.com")
Comment thread
DevinCLane marked this conversation as resolved.

const URLRegex =
/(https?:\/\/[^\s]+|www\.[^\s]+|[a-zA-Z0-9-]+\.[a-zA-Z]{2,}(?:[^\s]*))/g;

const LinkText = ({ children }) => {
if (typeof children !== "string")
throw new Error("LinkText can only accept a string as children");

const separated = children.split(URLRegex);

return separated.map((res, index) => {
if (URLRegex.test(res)) {
Copy link
Copy Markdown

@indifferentghost indifferentghost Aug 28, 2025

Choose a reason for hiding this comment

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

With a named capturing group we can better use our methods to get a predictable result. (I prefer ending early over gate control.

Suggested change
if (URLRegex.test(res)) {
const parsedUrl = URLRegex.exec(res);
if (parsedUrl === null) {
return res;
}

let href;

// Determine href and always enforce HTTPS
if (res.startsWith("https://")) {
href = res;
} else if (res.startsWith("http://")) {
href = res.replace(/^http:\/\//, "https://");
} else if (res.startsWith("www.")) {
href = `https://${res}`;
} else {
href = `https://${res}`;
}

// Convert Unicode domain to punycode for display
let displayURL;
try {
const urlObj = new URL(href);
displayURL = urlObj.toString(); // toString() converts hostname to punycode
} catch {
displayURL = href; // fallback if URL parsing fails
}

return (
<a
key={`${res}-${index}`}
href={href}
target="_blank"
rel="noreferrer"
className="text-blue-600 underline hover:text-blue-800"
>
{displayURL}
</a>
);
}
Copy link
Copy Markdown

@indifferentghost indifferentghost Aug 28, 2025

Choose a reason for hiding this comment

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

Since our check returns res early we don't need this closing bracket.

Suggested change
}

Copy link
Copy Markdown

@indifferentghost indifferentghost Aug 29, 2025

Choose a reason for hiding this comment

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

Here's the early returning check #641 (comment)

return res;
});
};

const EventModal = () => {
const { setEvents } = useEventsContext();
const modal = useModalContext();
Expand Down Expand Up @@ -94,7 +148,9 @@ const EventModal = () => {
</div>
<h3 className="mb-0">Description:</h3>{" "}
<div className="description break-words w-auto min-h-20 mb-2 p-2 border-solid border-black border-2 font-semibold rounded-xl bg-neutral-200/50">
<p>{modal.activeEvent.description}</p>
<p>
<LinkText>{modal.activeEvent.description}</LinkText>
Copy link
Copy Markdown

@indifferentghost indifferentghost Aug 28, 2025

Choose a reason for hiding this comment

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

If the LinkText isn't for user inserted content, we may have over-engineered, but it's okay.

</p>
</div>
<div>
{/* <section className="flex m-3 gap-1 font-semibold">
Expand All @@ -106,7 +162,17 @@ const EventModal = () => {
</section> */}
<section className="flex m-3 gap-1 font-semibold">
<IoLocationOutline className="mt-1" />{" "}
<span>Location: {modal.activeEvent.location}</span>
<span>
Location:{" "}
<a
href={modal.activeEvent.location}
target="_blank"
rel="noopener noreferrer"
className="text-blue-600 underline hover:text-blue-800"
>
{modal.activeEvent.location}
</a>
</span>
Comment on lines +165 to +175
Copy link
Copy Markdown

@indifferentghost indifferentghost Aug 28, 2025

Choose a reason for hiding this comment

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

Is this different from the LinkText component we built?

Copy link
Copy Markdown
Contributor Author

@dan-foley dan-foley Aug 29, 2025

Choose a reason for hiding this comment

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

Yes, I did that part first but wasn't able to do the same thing for the event description links. Thats where you came in and recommended the LinkText component.

</section>
{user ? (
<>
Expand Down