Skip to content

fix public view #34028

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.

Sign up for GitHub

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

Jump to bottom
Merged
szaimen merged 2 commits into from
Sep 13, 2022
Merged

fix public view #34028

szaimen merged 2 commits into from
Sep 13, 2022

Conversation

@szaimen
Copy link
Contributor

@szaimen szaimen commented Sep 12, 2022 •
edited
Loading

Close #33858
Close #33803

Signed-off-by: szaimen [email protected]

@szaimen szaimen added the 2. developing Work in progress label Sep 12, 2022
@szaimen
Copy link
Contributor Author

szaimen commented Sep 12, 2022

/compile /

@szaimen szaimen force-pushed the enh/33803/public-view branch from abdc077 to a83bf64 Compare September 12, 2022 12:04
@szaimen
Copy link
Contributor Author

szaimen commented Sep 12, 2022

/compile /

@szaimen szaimen added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Sep 12, 2022
@szaimen szaimen requested review from a team, CarlSchwan, PVince81, Pytal, juliusknorr, max-nextcloud and skjnldsv and removed request for a team September 12, 2022 12:26
@szaimen szaimen added this to the Nextcloud 25 milestone Sep 12, 2022
@szaimen szaimen marked this pull request as ready for review September 12, 2022 12:26
@szaimen szaimen force-pushed the enh/33803/public-view branch from c54f88e to 6d431c8 Compare September 12, 2022 13:31
@szaimen
Copy link
Contributor Author

szaimen commented Sep 12, 2022

/compile amend /

@max-nextcloud
Copy link
Contributor

max-nextcloud commented Sep 12, 2022

thanks for looking into this!
Haven't had time yet to try it out.

@szaimen szaimen force-pushed the enh/33803/public-view branch from 6d431c8 to 2432df5 Compare September 12, 2022 13:58
@szaimen
Copy link
Contributor Author

szaimen commented Sep 12, 2022

/compile amend /

@szaimen szaimen force-pushed the enh/33803/public-view branch from ca91803 to a922b12 Compare September 12, 2022 20:34
@szaimen
Copy link
Contributor Author

szaimen commented Sep 12, 2022

/compile amend /

skjnldsv
skjnldsv reviewed Sep 13, 2022
View reviewed changes
apps/files_sharing/css/public.scss

#imgframe img {
max-height: 100% !important;
max-height: calc(100vh - var(--header-height) - #{$footer-height} - #{$download-button-section-height}) !important;
Copy link
Member

@skjnldsv skjnldsv Sep 13, 2022

Choose a reason for hiding this comment

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

Oh wow 🙈

Copy link
Contributor Author

@szaimen szaimen Sep 13, 2022

Choose a reason for hiding this comment

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

Do you have better ideas? Other solutions didnt work in my testing...

PVince81
PVince81 approved these changes Sep 13, 2022
View reviewed changes
Copy link
Member

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

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

👍

skjnldsv
skjnldsv reviewed Sep 13, 2022
View reviewed changes
apps/files_sharing/css/public.scss
}

#imgframe video {
max-height: calc(100vh - var(--header-height) - #{$footer-height} - #{$download-button-section-height});
Copy link
Member

@skjnldsv skjnldsv Sep 13, 2022

Choose a reason for hiding this comment

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

Is it still relevant?
Considering we're using the viewer now to render single files shares?

Copy link
Contributor Author

@szaimen szaimen Sep 13, 2022

Choose a reason for hiding this comment

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

Yes, this was needed to make it work with any screen size...

Copy link
Member

@skjnldsv skjnldsv Sep 13, 2022

Choose a reason for hiding this comment

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

SHould it not be handled widely then? Make the viewer container takign the full size?
Not just video or img tags

Copy link
Contributor Author

@szaimen szaimen Sep 13, 2022

Choose a reason for hiding this comment

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

I didn't find other formats that would need fixing so only applied it to these.

Copy link
Contributor Author

@szaimen szaimen Sep 13, 2022

Choose a reason for hiding this comment

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

Btw: public audio playback is currently broken IIRC: #34027

Copy link
Contributor

@max-nextcloud max-nextcloud Sep 13, 2022

Choose a reason for hiding this comment

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

I'm working on a separate PR for audio playback here #34051 - would style that after this is merged.

@szaimen
fix public view
10ad6e1 
Signed-off-by: szaimen <[email protected]>
@szaimen szaimen force-pushed the enh/33803/public-view branch from 150441c to b664644 Compare September 13, 2022 13:00
@szaimen szaimen added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Sep 13, 2022
@szaimen
Copy link
Contributor Author

szaimen commented Sep 13, 2022

/compile amend /

@szaimen szaimen mentioned this pull request Sep 13, 2022
Merged
2 tasks
@nextcloud-command
Compile assets
335964d 
Signed-off-by: nextcloud-command <[email protected]>
@szaimen
Copy link
Contributor Author

szaimen commented Sep 13, 2022

CI failure unrelated

@szaimen szaimen merged commit b739dd5 into master Sep 13, 2022
@szaimen szaimen deleted the enh/33803/public-view branch September 13, 2022 14:08
@skjnldsv skjnldsv mentioned this pull request Sep 15, 2022
Merged
@szaimen szaimen mentioned this pull request Apr 17, 2023
Closed
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@skjnldsv skjnldsv skjnldsv left review comments

@PVince81 PVince81 PVince81 approved these changes

@max-nextcloud max-nextcloud max-nextcloud approved these changes

@juliusknorr juliusknorr Awaiting requested review from juliusknorr

@CarlSchwan CarlSchwan Awaiting requested review from CarlSchwan

@Pytal Pytal Awaiting requested review from Pytal

Assignees

No one assigned

Labels

4. to release Ready to be released and/or waiting for tests to finish

Projects

None yet

Milestone

Nextcloud 25

Development

Successfully merging this pull request may close these issues.

sharing view needs a bit of rework BUG: public link image share, footer overlaps with scrollbar:

6 participants

@szaimen @max-nextcloud @PVince81 @skjnldsv @nextcloud-command