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 script loading errors in ssbrowser.html and unauthorized.html #2383

Closed
wants to merge 1 commit into from

Conversation

serg-cymbaluk
Copy link
Contributor

@serg-cymbaluk serg-cymbaluk added WebUI Pull Request related to WebUI needs review Pull Request is waiting for a review labels Sep 24, 2018
@rcritten
Copy link
Contributor

So the fix was just to move the javascript out of the html file? Can you describe the reasoning for the fix in the commit message?

@stanislavlevin
Copy link
Contributor

Hello, @serg-cymbaluk
My apologies, this was my mistake (checked at dev mode only).

Please take a look at last commit:
https://github.com/stanislavlevin/freeipa/commits/fix_js_at_production

It completely fixes the issue.

@serg-cymbaluk
Copy link
Contributor Author

@rcritten

So the fix was just to move the javascript out of the html file? Can you describe the reasoning for the fix in the commit message?

We can't import modules in HTML-file because it requires js file in the runtime. But when we import module from other module this dependency is resolved in compile-time.
So idea is that all pages have own compiled js file with all dependencies inside.
But at this moment I am inclined to accept @stanislavlevin's patch because it's simpler one.

@serg-cymbaluk
Copy link
Contributor Author

serg-cymbaluk commented Sep 25, 2018

Please take a look at last commit:
https://github.com/stanislavlevin/freeipa/commits/fix_js_at_production

@stanislavlevin, please create new PR if it is final edits.

@stanislavlevin
Copy link
Contributor

#2387 opened.
Excuse me one more time.

@freeipa-pr-ci freeipa-pr-ci added the needs rebase Pull Request cannot be automatically merged - needs to be rebased label Sep 26, 2018
@tiran tiran added the ipa-4-7 label Sep 26, 2018
@tiran
Copy link
Member

tiran commented Sep 26, 2018

@stanislavlevin This PR needs rebase, too.

@stanislavlevin
Copy link
Contributor

@stanislavlevin This PR needs rebase, too.

It does not belong to me ;-)
The issue was fixed at #2387 (already pushed).

About this PR, in my opinion, it is no longer needed.

@tiran
Copy link
Member

tiran commented Sep 26, 2018

Sorry @stanislavlevin

@serg-cymbalu, what about this PR? Is it still necessary?

@serg-cymbaluk
Copy link
Contributor Author

@tiran

what about this PR? Is it still necessary?

It is already not actual.

@serg-cymbaluk serg-cymbaluk added rejected Pull Request has been rejected and removed needs review Pull Request is waiting for a review labels Sep 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs rebase Pull Request cannot be automatically merged - needs to be rebased rejected Pull Request has been rejected WebUI Pull Request related to WebUI
Projects
None yet
5 participants