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

fix(images): use github/repo as hosting #1125

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

VIRUXE
Copy link

@VIRUXE VIRUXE commented May 31, 2024

Images are now hosted with GitHub and pages use their relative URLs.

Not sure if this structure would be acceptable, but am open to suggestions.

VIRUXE added 4 commits May 31, 2024 19:02
Images are now hosted with GitHub and pages use their relative URLs.

These pages all seem to be about csharp. Not sure how to fix for the other languages.
@PsychoShock
Copy link
Contributor

While I see the big effort, I think just inserting images instead of imgur links wouldnt be the best. Maybe size vs format won't fit exactly. Some tweaks in the UI need maybe to be done. Have you try to build docs in local to see?

I also think that type of structure is fine since it's easy for anyone to change it. Maybe change the images name with a "standart" for people that will contribute in the future to follow. I would recommand maybe image description (short version). E.g. for SetCamSplineSmoothingStyle it would be graphic_details rather than Lj97T3F.

Anyways that's just my personnal opinion, good job :)

@VIRUXE
Copy link
Author

VIRUXE commented Jun 1, 2024

Have you try to build docs in local to see?

I have previewed every page in vscode's markdown preview extension. Thus why I made some adjustments to the formatting, so images actually display and use the next line for rendering.

Maybe change the images name with a "standart" for people that will contribute in the future to follow. I would recommand maybe image description (short version). E.g. for SetCamSplineSmoothingStyle it would be graphic_details rather than Lj97T3F.

It would be fine to rename them at this point, since I already took the time to do all this. But I would like for this to be reviewed first, before doing further work.

Maybe @AvarianKnight could review?

@VIRUXE VIRUXE changed the title fix(images): use repo images fix(images): use github/repo as hosting Jun 1, 2024
@PsychoShock
Copy link
Contributor

PsychoShock commented Jun 1, 2024

I have previewed every page in vscode's markdown preview extension. Thus why I made some adjustments to the formatting, so images actually display and use the next line for rendering.

Vscode preview vs actual docs UI are not the same. You can build in localhost.

For the rest, before doing a big work like this it's always better to ask. Based on @4mmonium comment here (like you know), they are working in a solution wich not might be Github.

And for the code it-self, tweaks might need to be do for the correct format and size on every image.

Copy link
Collaborator

@AvarianKnight AvarianKnight left a comment

Choose a reason for hiding this comment

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

I'm unsure on what the status is of getting image links back, but this is a decent stop gap for now, and if needed can be reverted whenever the cdn stuff gets sorted out by Cfx.

OBJECT/IsPointInAngledArea.md Outdated Show resolved Hide resolved
VEHICLE/SetVehicleEnveffScale.md Outdated Show resolved Hide resolved
VEHICLE/SetVehicleEnveffScale.md Outdated Show resolved Hide resolved
VEHICLE/SetVehicleEnveffScale.md Outdated Show resolved Hide resolved
VEHICLE/SetVehicleEnveffScale.md Outdated Show resolved Hide resolved
@VIRUXE
Copy link
Author

VIRUXE commented Jul 26, 2024

Apologies for the time taken to correct this.

I have corrected the referenced files @AvarianKnight

@VIRUXE VIRUXE requested a review from AvarianKnight July 26, 2024 23:02
@AvarianKnight AvarianKnight added the Draft Work in progress PR. Remove label once PR is ready for review. label Aug 7, 2024
@AvarianKnight
Copy link
Collaborator

Going to mark this as a draft for now, we're discussing on how this should be handled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Draft Work in progress PR. Remove label once PR is ready for review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants