Skip to content

Conversation

@MorrisJobke
Copy link
Member

This type of check is also used some lines above 🙈

* checks if the user is on the login page or not instead of check if the user is logged in
* fixes #3207

Signed-off-by: Morris Jobke <hey@morrisjobke.de>
@mention-bot
Copy link

@MorrisJobke, thanks for your PR! By analyzing the history of the files in this pull request, we identified @LukasReschke, @rullzer and @bartv2 to be potential reviewers.

@skjnldsv
Copy link
Member

Works well, but I don't really like the way we do that.
Couldn't we create a function in oc_util for other uses too?

Like is_publicPage() :)

@MorrisJobke
Copy link
Member Author

Works well, but I don't really like the way we do that.
Couldn't we create a function in oc_util for other uses too?

Like is_publicPage() :)

We usually don't want to do that, because it would encourage people to use it that way instead of listening to the "public page hook" and do the stuff in this hook.

@nickvergessen
Copy link
Member

Also public page is not the right thing. Public share pages, public calendars and public call pages should all use the compiled version instead of recompiling all the time?

@skjnldsv
Copy link
Member

@nickvergessen there is no compilation "all the time" only once or if the original scss file has changed :)
So in a production env, it shouldn't happens.

@skjnldsv
Copy link
Member

We usually don't want to do that, because it would encourage people to use it that way instead of listening to the "public page hook" and do the stuff in this hook.

Okay, I see.
I need to see if I cannot directly change the way we handle things on the base login template.
Maybe we can avoid all of this! :)

@MorrisJobke
Copy link
Member Author

Can we get this in - otherwise the share page is broken. This just fixes the public share page and doesn't affect what is when compiled into CSS ;)

@skjnldsv
Copy link
Member

@MorrisJobke Sorry for taking so long, i wanted to test if we could directly pass a variable trough the template generator to exclude the 'addStyle' functions in template.php

With that we would have been able to just ignore some css files.
But I think it's better to just ignore the scss generation, even if i dislike what we're doing right now, seems like bad coding! 🙈 🙊 🙉

Copy link
Member

@schiessle schiessle left a comment

Choose a reason for hiding this comment

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

tested... works!

@schiessle schiessle merged commit f469b3e into master Jan 25, 2017
@schiessle schiessle deleted the fix-public-page-style branch January 25, 2017 10:45
@ChristophWurst
Copy link
Member

I guess this doesn't covery 2FA pages:
bildschirmfoto von 2017-01-30 14-10-02

@nickvergessen
Copy link
Member

See https://github.com/nextcloud/notifications/blob/master/lib/AppInfo/Application.php#L58 for help. I don't understand why the page is white thou, it should just not be cached from my pov.

@skjnldsv
Copy link
Member

Will take a look after.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Public page styles are broken on

7 participants