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

Add “Resources” section in the Airflow website #233 #1062

Merged
merged 4 commits into from
Sep 30, 2024

Conversation

gdivya6028
Copy link
Contributor

Add “Resources” section in the Airflow website #233

image

@gdivya6028
Copy link
Contributor Author

pre-commit issues fixed

@potiuk
Copy link
Member

potiuk commented Sep 1, 2024

I am not sure if adding the book to resources is a good idea. Generally whatever we publish (except Ecosystem page) should be PMC "owned" and "controlled" and the book is something written independently (even if one of the authors is a PMC members) -sp I think the right place to mention the book is "ecosystem" page.

The "promo materials" though is a very good idea to add.

@gdivya6028
Copy link
Contributor Author

@potiuk i will work on the feedback .. could you please review sticky sidebar implementation for "Community" page as well. It is requested on the same issue #233

  • Added sticky sidebar on scroll only for desktop, doesn't reflect on mobile or tablet screens.
sidebar_compressed.mp4

@potiuk
Copy link
Member

potiuk commented Sep 1, 2024

I am not good on the web side of it so maybe someone else.will, but if it works I see no reason for us not.merging it

@gdivya6028 gdivya6028 force-pushed the add-resources-section-and-sidebar branch from 25bf8d7 to 9c39416 Compare September 1, 2024 16:23
@gdivya6028
Copy link
Contributor Author

  • Resources section is added in Community page
  • Apache Airflow book updated in Ecosystem page
  • Sticky sidebar for navigation is added in Community page for desktop view
233.PR.changes_compressed.mp4

@potiuk
Copy link
Member

potiuk commented Sep 9, 2024

can someone more familiar with javascript/css etc. review it as well @pierrejeambrun @jscheffl @bbovenzi ?

@bbovenzi
Copy link
Contributor

Could we undo the whitespace changes to html and css? It's hard to tell what actually changed.

Copy link
Contributor

@jscheffl jscheffl left a comment

Choose a reason for hiding this comment

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

I checked the diff w/o whitespace...
image

Is the build testable in a different URL?
Actually this PR mixed a bit of sticky layout changes with adding a menu entry. Next time might be easier to separate.

From point of code view... seems reasonable.

@bbovenzi
Copy link
Contributor

I checked the diff w/o whitespace... image

Is the build testable in a different URL? Actually this PR mixed a bit of sticky layout changes with adding a menu entry. Next time might be easier to separate.

From point of code view... seems reasonable.

Oh I never noticed that option before.

This generally looks good to me. It would still be best not to contribute whitespace changes.

@gdivya6028
Copy link
Contributor Author

@potiuk / @jscheffl / @bbovenzi, Could you let me know if there are any actions required from my end here ?

@potiuk
Copy link
Member

potiuk commented Sep 12, 2024

@potiuk / @jscheffl / @bbovenzi, Could you let me know if there are any actions required from my end here ?

Removing the whitespace changes?

@jscheffl
Copy link
Contributor

@potiuk / @jscheffl / @bbovenzi, Could you let me know if there are any actions required from my end here ?

Removing the whitespace changes?

I would not insist on it. Would be better but.... LGTM

@bbovenzi bbovenzi merged commit c4b25a1 into apache:main Sep 30, 2024
1 check passed
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.

4 participants