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

#71 updated current and prev tech leads; replaced raw html with the react components using .mdx #144

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

Conversation

Anjali0407-git
Copy link
Collaborator

@Anjali0407-git Anjali0407-git commented Nov 12, 2024

Fixes #71

@Anjali0407-git Anjali0407-git linked an issue Nov 12, 2024 that may be closed by this pull request
@Anjali0407-git Anjali0407-git self-assigned this Nov 13, 2024
Copy link
Collaborator

@kungfuchicken kungfuchicken left a comment

Choose a reason for hiding this comment

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

I have one small change, and I want to check my assumptions.

First, I think it makes sense to update the profile picture for Dr. Holdener and myself using the same Profile component, but I think it might make more sense to directly replace the <div> tag with it. What do you think?

That leads to my next question, a check on my assumptions. I was originally thinking of directly replacing <div>-based profiles with the Profile component directly in the .mdx file. My thinking was it is obvious what is doing what, and making updates is just editing a slightly fancy markdown file. Your approach separates the MDX and component as a presentation of a separate set of data. That's still pretty clean, and updates are just as quick and easy. It forces a preservation of the overall layout and structure of the page, too. My only concern is that it might add to the complexity, and therefore add to potential confusion when we go back to update it. What do you think?

As I'm looking at the overall page layout there are some other elements that probably need restructuring, too. I think it's worth getting this done before moving on to those, though.

@Anjali0407-git
Copy link
Collaborator Author

I think replacing the <div> tags with Profile component won't actually reduce the number of lines of code we use. Also, since we'll need to keep adding new student profiles every semester, the way we're doing it now might be easier. As the Current Tech leads, Previous Tech leads and Developers does not have the individual anchor tags to worry about.
However, for important roles like the Leadership Team or SLU research Tech Leads, using the Profile component seem like the best option because it has your names as the title which is an anchor tag. Not sure if we can replicate the same behaviour using React

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.

React component for profile pictures
2 participants