Skip to content

Introduction to javafx.animation #97

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

Merged
merged 7 commits into from
May 31, 2024

Conversation

SquidXTV
Copy link
Contributor

About

Adding new Introduction to the javafx.animation package article.

To-Do

While implementing the markdown into the application, I noticed some more stuff that needs to be done first.

Author

So I have skipped the author part for now. Should I just create a new author in /data/authors.yaml myself? Anything I need to look out for?

Java Docs links

I used /data/javafxdoc.json and I hope that is fine. I tried to change the version as it is still at 19, but that didn't work. It only works if I change the version in /data/javadoc.json as well. Seems like unintended behavior, as they shouldn't be related. Should I just ignore it?

GIFs

First, I noticed that the GIFs most likely need some border or similar, otherwise it looks weird. What would you recommend?
Also, I would prefer to somehow sync the Interpolator GIFs. How should I do that? Should I just merge them into one bigger GIF?
That would work because all GIFs have a title to distinguish them and the same duration. Syncing would make it easier for the reader to see the difference between the interpolator visually.

GIF example

Copy link

Thank you for your pull request and welcome to our community! To contribute, please sign the Oracle Contributor Agreement (OCA).
The following contributors of this PR have not signed the OCA:

To sign the OCA, please create an Oracle account and sign the OCA in Oracle's Contributor Agreement Application.

When signing the OCA, please provide your GitHub username. After signing the OCA and getting an OCA approval from Oracle, this PR will be automatically updated.

If you are an Oracle employee, please make sure that you are a member of the main Oracle GitHub organization, and your membership in this organization is public.

@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Required At least one contributor does not have an approved Oracle Contributor Agreement. label May 17, 2024
@SquidXTV
Copy link
Contributor Author

Thank you for your pull request and welcome to our community! To contribute, please sign the Oracle Contributor Agreement (OCA). The following contributors of this PR have not signed the OCA:

To sign the OCA, please create an Oracle account and sign the OCA in Oracle's Contributor Agreement Application.

When signing the OCA, please provide your GitHub username. After signing the OCA and getting an OCA approval from Oracle, this PR will be automatically updated.

If you are an Oracle employee, please make sure that you are a member of the main Oracle GitHub organization, and your membership in this organization is public.

Uh one quick question, might be wrong place though. The account creation is asking me for required fields like "Job Title", "Work Phone" and "Company Name". What should I put there? I am still going to school, so I can't really answer them.

@danthe1st
Copy link
Contributor

danthe1st commented May 17, 2024

So I have skipped the author part for now. Should I just create a new author in /data/authors.yaml myself? Anything I need to look out for?

I don't know whether this is required but that has certainly been done for the previous articles contributed by non-Oracle employees (And I think having an author article is at least preferred, especially since I think that's necessary for it to display information about it being contributed under the UPL).

I used /data/javafxdoc.json and I hope that is fine. I tried to change the version as it is still at 19, but that didn't work. It only works if I change the version in /data/javadoc.json as well. Seems like unintended behavior, as they shouldn't be related. Should I just ignore it?

I just want to mention that this is different in this repository and the version on https://dev.java.
For example, https://dev.java/learn/javafx/structure/ uses the JavaFX 21 Javadoc (it's probably fine if you just do it as it's done in that article which I think should be in this repository) while e.g. https://dev.java/learn/records/ references Java 22 Javadocs.

Disclaimer: I am not an Oracle employee, I'm an outside contributor just as you are.

Copy link
Contributor

@danthe1st danthe1st left a comment

Choose a reason for hiding this comment

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

Just posting a bit of feedback. Again, I am not an Oracle employee so these are just suggestions that you don't have to follow.

Regarding your question about synchronizing GIFs, do all of them have the same duration?

@SquidXTV
Copy link
Contributor Author

Just posting a bit of feedback. Again, I am not an Oracle employee so these are just suggestions that you don't have to follow.

Thanks for the feedback, gives me time to work on something in the meantime.

Regarding your question about synchronizing GIFs, do all of them have the same duration?

Yes, they all have the same duration.

@SquidXTV SquidXTV marked this pull request as ready for review May 19, 2024 15:42
Copy link
Contributor

@danthe1st danthe1st left a comment

Choose a reason for hiding this comment

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

Just a few other things I noticed.

@carimura
Copy link
Member

Hi @SquidXTV, thanks for the contribution. We're working through some feedback and will get that back to you shortly. Before we go any further, we'll need the OCA signed. First, please only continue if you are over 18. For title, you can put student. For company, you can also put student, or your school name. For the rest, you can put your contact info. None of this is public. Thanks!

@carimura
Copy link
Member

Hi @danthe1st as usual thanks for the feedback. Please hold off on any further feedback until we can get the OCA signed and make sure the overall goals of the article are met.

@SquidXTV
Copy link
Contributor Author

Hi @SquidXTV, thanks for the contribution. We're working through some feedback and will get that back to you shortly. Before we go any further, we'll need the OCA signed. First, please only continue if you are over 18. For title, you can put student. For company, you can also put student, or your school name. For the rest, you can put your contact info. None of this is public. Thanks!

Okay thanks for the response and yes I am currently 18 so that should be fine. Going to sign the OCA as soon as possible.

ammbra
ammbra previously requested changes May 20, 2024
Copy link
Collaborator

@ammbra ammbra left a comment

Choose a reason for hiding this comment

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

Hello,

I noticed that is only in draft, but given the conversations going on, I took a look and provided a few points for you to think of.

Copy link
Contributor

@nipafx nipafx left a comment

Choose a reason for hiding this comment

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

Hey @SquidXTV! 👋🏾 Thank you for the submission. As I often do during a review, I made a lot of detailed comments and suggestions. Don't let that demoralize you, this is just the finishing touch we need to get this over the finishing line.

Copy link

Thank you for signing the OCA.

@oracle-contributor-agreement oracle-contributor-agreement bot added OCA Verified All contributors have signed the Oracle Contributor Agreement. and removed OCA Required At least one contributor does not have an approved Oracle Contributor Agreement. labels May 21, 2024
Copy link
Contributor

@nipafx nipafx left a comment

Choose a reason for hiding this comment

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

Two more changes and it can be shipped - I'll apply them right away.

@nipafx
Copy link
Contributor

nipafx commented May 31, 2024

@carimura Merge any time you want. 🚀

@SquidXTV Thank you for all your hard work during this lengthy process. 😊 I think it improved the article and it's really good now. 👍🏾

@carimura carimura dismissed ammbra’s stale review May 31, 2024 13:01

All changes addressed!

@carimura
Copy link
Member

ok it appears everything is resolved here and we've got a few approvals, let's get this one done! @SquidXTV would you like to do the honors in merging? (if you can do today, otherwise, I'll handle later today) :)

@danthe1st
Copy link
Contributor

@carimura I think only people with write access to the repository can merge.
image

@carimura
Copy link
Member

ah fair enough.

@carimura
Copy link
Member

well then the honor is mine.... thanks @SquidXTV @nipafx and @danthe1st!

@carimura carimura merged commit 1b37f5a into java:main May 31, 2024
1 check passed
@carimura
Copy link
Member

carimura commented May 31, 2024

I just now realized we're missing an author entry.... @SquidXTV care to open a new PR for that? https://github.com/java/devjava-content/blob/main/app/data/authors.yaml

@SquidXTV
Copy link
Contributor Author

I just now realized we're missing an author entry.... @SquidXTV care to open a new PR for that? https://github.com/java/devjava-content/blob/main/app/data/authors.yaml

I tried working on it but if I want to preview my or any others author page it just gives me a 404:

Cannot GET /author/ConnorSchweigh%C3%B6fer

Different example:

Cannot GET /author/VenkatSubramaniam

@SquidXTV
Copy link
Contributor Author

I tried working on it but if I want to preview my or any others author page it just gives me a 404:

Ah well I tried using docker instead and that worked. Might be worth to create an issue about it.

@Just-some-one
Copy link

thanks @SquidXTV for the tutorial

i would like to mention that all links on the page are dead

@SquidXTV
Copy link
Contributor Author

thanks @SquidXTV for the tutorial

i would like to mention that all links on the page are dead

Nothing I can do about that I guess. Maybe check #104

@Just-some-one
Copy link

Just-some-one commented May 31, 2024

i think i got the issue
on the javafxdoc.json file
all the elements here without the .html

starting from
"AnimationPackageSummary": "javafx.graphics/javafx/animation/package-summary",

to

"AnimationTimer": "javafx.graphics/javafx/animation/AnimationTimer"

add the .html at the end of each value and the problem will be fixed

try to open any link there and you will find it missing the html add it and it will work

also pay attention to the one that end with id to certain section of the linked page

like this one

"Interpolator.EASE_IN": "javafx.graphics/javafx/animation/Interpolator#EASE_IN"

so add the .html after Interpolator

hope that help and have a nice day :)

@SquidXTV
Copy link
Contributor Author

i think i got the issue on the javafxdoc.json file all the elements here without the .html

starting from "AnimationPackageSummary": "javafx.graphics/javafx/animation/package-summary",

to

"AnimationTimer": "javafx.graphics/javafx/animation/AnimationTimer"

add the .html at the end of each value and the problem will be fixed

try to open any link there and you will find it missing the html add it and it will work

also pay attention to the one that end with id to certain section of the linked page

like this one

"Interpolator.EASE_IN": "javafx.graphics/javafx/animation/Interpolator#EASE_IN"

so add the .html after Interpolator

hope that help and have a nice day :)

Ah thanks, thats very helpful.
This wasn't a problem with the original links I used, but they changed it when shipping to prod.
Going to create a PR for that soon.

@carimura
Copy link
Member

oh shoot I'll fix this now thanks for noticing

@Just-some-one
Copy link

thanks for you @SquidXTV @carimura

@carimura
Copy link
Member

alright fix has been deployed and changes sync'ed to this repo here: #109

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCA Verified All contributors have signed the Oracle Contributor Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants