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

Add guest content component #3369

Merged
merged 1 commit into from
Oct 19, 2022
Merged

Conversation

ChristophWurst
Copy link
Contributor

For pages like

Bildschirmfoto vom 2022-10-17 10-42-20

This could be used for login, login flow, 2FA, OAuth redirect pages, etc.

In a future version I could also image that there is a "fullscreen" or size prop to make the container expand to the available width/height.

@ChristophWurst ChristophWurst added enhancement New feature or request 3. to review Waiting for reviews component Component discussion and/or suggestion labels Oct 17, 2022
@ChristophWurst ChristophWurst self-assigned this Oct 17, 2022
@JuliaKirschenheuter
Copy link
Contributor

In a future version I could also image that there is a "fullscreen" or size prop to make the container expand to the available width/height.

Sounds good!

@JuliaKirschenheuter
Copy link
Contributor

JuliaKirschenheuter commented Oct 17, 2022

@ChristophWurst
Copy link
Contributor Author

Because there is no such component at the moment.

@ChristophWurst
Copy link
Contributor Author

Acute need for the component is triggered by nextcloud/mail#7430.

Copy link
Contributor

@GretaD GretaD left a comment

Choose a reason for hiding this comment

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

Code looks fine, i didnt test it with an app

@ChristophWurst
Copy link
Contributor Author

@JuliaKirschenheuter is there any issue with the current solution?

Copy link
Contributor

@st3iny st3iny left a comment

Choose a reason for hiding this comment

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

I like the idea!

However, it doesn't work out of the box with components inside our regular content container. Have a look at an example from Calendar:

At the top

image

Scrolled a bit down

image

IMO, NcGuestContent needs at least margin: 0 auto to center it and maybe height: fit-content to make it grow.

EDIT: The outer content container might also need changes to allow scrolling and prevent clipping at the bottom (taken from calendar/src/views/Appointments/Booking.vue):

#content {
	// Enable scrolling
	overflow: auto;

	// Fix box being cutoff at the bottom
	margin-bottom: 0;
	height: calc(var(--body-height) + var(--body-container-margin));
}

@ChristophWurst
Copy link
Contributor Author

MO, NcGuestContent needs at least margin: 0 auto to center it and maybe height: fit-content to make it grow.

While you already have your fancy setup to check this 😈, could you push those styles to my branch?

@st3iny
Copy link
Contributor

st3iny commented Oct 18, 2022

MO, NcGuestContent needs at least margin: 0 auto to center it and maybe height: fit-content to make it grow.

While you already have your fancy setup to check this smiling_imp, could you push those styles to my branch?

Sure thing

Copy link
Contributor

@st3iny st3iny left a comment

Choose a reason for hiding this comment

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

Screenshots from Calendar appointments feature

scroll-guest-content.webm

Screenshot 2022-10-18 at 12-10-38 Nextcloud

Signed-off-by: Christoph Wurst <[email protected]>
@ChristophWurst ChristophWurst force-pushed the enhancement/guest-content-component branch from 9cf71ae to 2e9bc89 Compare October 19, 2022 07:34
@ChristophWurst ChristophWurst added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Oct 19, 2022
@st3iny st3iny merged commit 40b7417 into master Oct 19, 2022
@st3iny st3iny deleted the enhancement/guest-content-component branch October 19, 2022 12:27
@skjnldsv skjnldsv mentioned this pull request Nov 15, 2022
}
</style>

<style lang="scss">
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not scoped and therefore breaks files app, settings, basically any page layout by missing the bottom border:
grafik

grafik

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish component Component discussion and/or suggestion enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants