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

display the upgrade progress. #4574

Merged
merged 1 commit into from
Aug 29, 2024

Conversation

zyhfish
Copy link
Contributor

@zyhfish zyhfish commented Aug 28, 2024

No description provided.

@sbwalker sbwalker merged commit d718969 into oqtane:dev Aug 29, 2024
1 check passed
@sbwalker
Copy link
Member

sbwalker commented Aug 29, 2024

@zyhfish if you search the code base for "bootstrap/5" you will find a number of external references to Bootstrap:

  • Install Wizard
  • Blazor Theme
  • Oqtane Theme (for JavaScript - not CSS)

These all use https://cdnjs.cloudflare.com as the source (including the Integrity attribute)

The new app_offline.bak uses https://cdn.jsdelivr.net (without Integrity attribute)

For consistency and ease in maintaining the framework, could you please abstract these references into the Constants.cs:

BootstrapScriptUrl = "https://cdnjs.cloudflare.com/ajax/libs/bootstrap/5.3.3/js/bootstrap.bundle.min.js"
BootstrapScriptIntegrity = "sha512-7Pi/otdlbbCR+LnW+F7PwFcSDJOuUJB3OxtEHbg4vSMvzvJjde4Po1v4BR9Gdc9aXNUNFVUY+SK51wWT8WF0Gg=="
BootstrapStylesheetUrl = "https://cdnjs.cloudflare.com/ajax/libs/bootstrap/5.3.3/css/bootstrap.min.css"
BootstrapStylesheetIntegrity = "sha512-jnSuA4Ss2PkkikSOLtYs8BlYIeeIK1h99ty4YfvRPAlzr377vr3CXDb7sb7eEEBYjDtcYj+AjBH3FLv5uSJuXg=="

And reference the constant in all of the various locations above. This will allow us to update the Bootstrap reference in 1 place in the future.

@zyhfish zyhfish deleted the task/display-upgrade-progress branch August 29, 2024 05:28
@zyhfish
Copy link
Contributor Author

zyhfish commented Aug 29, 2024

Hi @sbwalker ,
thanks so much for review the changes, one issue is that the Oqtane.Update project is a standalone project and it don't have any reference to the Oqtane.Shared project so it can't use the Constants class, should we add the reference?

Thanks,
Ben

@sbwalker
Copy link
Member

I don't think it's a problem to include the Shared reference as the Updater assembly is located in the /bin - however the app_offline.bak is part of the Server project... and the fact that it is an HTML file means that it cannot reference constants anyways right?

@zyhfish
Copy link
Contributor Author

zyhfish commented Aug 29, 2024

This will not be a problem, we can set the bootstrap css URL to a token and be replaced with the Constants value in the update project.

@zyhfish
Copy link
Contributor Author

zyhfish commented Aug 29, 2024

submitted #4577 to handle this change request.

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.

2 participants