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 site footer #567

Closed
wants to merge 4 commits into from
Closed

Fix site footer #567

wants to merge 4 commits into from

Conversation

czullu
Copy link
Contributor

@czullu czullu commented Jan 7, 2022

this change overlaps with open pull request #498 / #562 but includes the handling for
ShowSiteTitle = !web.HideTitleInHeader
as well. i did try to align for easier merging

Christian Zuellig added 2 commits January 7, 2022 17:17
add Header HideTitleInHeader to ShowSiteTitle mapping and new LayoutTypes also to provisioning
@jansenbe
Copy link
Contributor

@czullu : I'm not yet merging this one. The key reason is (and it's not just due to your changes) is that actually we should use the SetChromeOptions over updating the respective web properties as updating these web properties requires full control on the site and therefore might fail. I've re-implemented branding in PnP Core SDK (see https://pnp.github.io/pnpcore/using-the-sdk/branding-intro.html and https://github.com/pnp/pnpcore/tree/dev/src/sdk/PnP.Core/Model/SharePoint/Branding) without updating the web. This approach is aligned to what is offered by the UI and can maybe serve as inspiration. Possibly some of the PnP Core SDK logic can be reused/extended for this purpose.

Also adding @fzbm and @PedroMordeP as they've been active on site header/footer configuration

@jansenbe jansenbe self-assigned this Jan 12, 2022
@czullu
Copy link
Contributor Author

czullu commented Jan 12, 2022

Ok - was not aware that there is a difference but can change to use pnpcorecontext as in ObjectClientSidePages.
i can do the change next weekend and let you know.

@eduardpaul
Copy link
Contributor

Hi, will this PR fix this #646 ? Sorry opened the issue without checking active PR's.

@jansenbe
Copy link
Contributor

closing as too old

@jansenbe jansenbe closed this May 23, 2024
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.

None yet

3 participants