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

Remove links from outreachy project description #1764

Merged
merged 8 commits into from Nov 15, 2023
Merged

Conversation

Solar-Rays
Copy link
Contributor

Added a space in the url causing a spill over on mobile

@Solar-Rays
Copy link
Contributor Author

@sabine please review the fix and advise further

Comment on lines 175 to 179
(https://v3.ocaml.org/docs/metaprogramming#attributes-and-derivers) and
(https://v3.ocaml.org/docs/metaprogramming
#attributes-and-derivers) and
extension points
(https://v3.ocaml.org/docs/metaprogramming#extension-nodes-and-extenders).
(https://v3.ocaml.org/docs/metaprogramming
#extension-nodes-and-extenders).
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think, since these descriptions are just plain text, they cannot contain URLs in the first place. So the best thing here would be to remove the 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.

That is fine then.

<% if String.length project.description > 240 then ( %>
<div :class="is_shown ? '' : 'relative max-h-20 overflow-hidden'">
<div class="absolute bottom-0 h-10 left-0 right-0 bg-gradient-to-t from-white"></div>
<p> <%s project.description %> </p>
<p class="overflow-fix"> <%s project.description %> </p>
Copy link
Collaborator

Choose a reason for hiding this comment

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

overflow-fix doesn't seem defined anywhere and I don't think this file needs to be changed when we remove the offending links from the outreachy descriptions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, I added it during debugging and forgot to remove it when I found the bug. Do you need me to take it out or is that already done?

Copy link
Collaborator

Choose a reason for hiding this comment

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

when you look at the "Files changed" tab, you'll see that two files are being edited by this PR. Best is to check out the old version of outreachy.eml, so the PR doesn't make any changes to this file.

@Solar-Rays
Copy link
Contributor Author

Solar-Rays commented Nov 15, 2023

I have restored outreachy.eml to the previous state. I only made the change to the data file with the rogue url. You can check now please.

Copy link
Collaborator

@sabine sabine left a comment

Choose a reason for hiding this comment

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

Thank you @Solar-Rays! Merging this now. 👍

@sabine sabine changed the title Mobile Remove links from outreachy project description Nov 15, 2023
@sabine sabine merged commit 813ae6e into ocaml:main Nov 15, 2023
1 check passed
sabine pushed a commit to sabine/ocaml.org that referenced this pull request Nov 24, 2023
- fixes overflowing div on outreachy page
sabine pushed a commit to balat/ocaml.org that referenced this pull request Dec 12, 2023
- fixes overflowing div on outreachy 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.

None yet

2 participants