Skip to content

fix right-click img tags - #36847

Merged
szaimen merged 1 commit into
masterfrom
enh/noid/fix-right-click-images
Mar 2, 2023
Merged

fix right-click img tags#36847
szaimen merged 1 commit into
masterfrom
enh/noid/fix-right-click-images

Conversation

@szaimen

@szaimenszaimen commented Feb 24, 2023

Copy link
Copy Markdown
Contributor

Follow-up to #35484

Fix#36832
Fix#36154

BeforeAfter
imageimage

@szaimenszaimen added bug 2. developing Work in progress labels Feb 24, 2023
@szaimenszaimen added this to the Nextcloud 26 milestone Feb 24, 2023
@szaimen

Copy link
Copy Markdown
ContributorAuthor

/compile amend /

@nextcloud-command
nextcloud-commandforce-pushed the enh/noid/fix-right-click-images branch from a951605 to f643af3CompareFebruary 24, 2023 10:26
@szaimenszaimen closed this Mar 1, 2023
@szaimen
szaimen deleted the enh/noid/fix-right-click-images branch March 1, 2023 13:43
rotdrop added a commit to rotdrop/nextcloud-server that referenced this pull request Mar 1, 2023
@rotdrop

Copy link
Copy Markdown
Contributor

Are you sure that this fixes #36154? We have still the width and height of the image tag set to 16px and its padding set to 14px. And box-sizing border-box. But this means that regardless of your change the following lines:

server/core/css/apps.scss

Lines 1144 to 1147 in 86c102e

> img {
width: $popovericon-size;
padding: #{math.div($popoveritem-height-$popovericon-size, 2)};
}

will still result in total size of 28px ("border-box" means that content + padding make up for the size, doesn't it?)

I have only tested your changes with NC25 and it does not seem to change anything. BTW, also the "Edit locally" menu item is affected and does not show the icon.

Is maybe the ":not()" wrong? At least #36154 talks about the very filesActionMenu and the :not() excludes it. So your CSS rules changes do not affect the filesActionMenu. If I remove the not(...) then the issues with the files-action menu described in #36154 seem to be fixed for NC 25.

rotdrop added a commit to rotdrop/nextcloud-server that referenced this pull request Mar 1, 2023
rotdrop added a commit to rotdrop/nextcloud-server that referenced this pull request Mar 1, 2023
@szaimen
szaimen restored the enh/noid/fix-right-click-images branch March 2, 2023 08:31
@szaimenszaimen reopened this Mar 2, 2023
@szaimen
szaimenforce-pushed the enh/noid/fix-right-click-images branch 2 times, most recently from e1c50fe to a415357CompareMarch 2, 2023 08:35
@szaimen

Copy link
Copy Markdown
ContributorAuthor

/compile amend /

Signed-off-by: Simon L <szaimen@e.mail.de>
Signed-off-by: nextcloud-command <nextcloud-command@users.noreply.github.com>
@nextcloud-command
nextcloud-commandforce-pushed the enh/noid/fix-right-click-images branch from a415357 to 2715f40CompareMarch 2, 2023 08:58
@szaimenszaimen added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Mar 2, 2023
@szaimen
szaimen requested review from a team, Pytal, artonge, rotdrop and skjnldsv and removed request for a teamMarch 2, 2023 09:07
@szaimen

Copy link
Copy Markdown
ContributorAuthor

/backport to stable25

@rotdroprotdrop left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This seems to fix the issue. However, I suppose the lines

server/core/css/apps.scss

Lines 1144 to 1147 in 86c102e

> img {
width: $popovericon-size;
padding: #{math.div($popoveritem-height-$popovericon-size, 2)};
}

should be revisited as my impression is that these settings do not make sense with box-sizing border-box. Just mentioned it here because that rule makes the reviewed changes "necessary" and maybe are bogus. But this is probably something for a separate new issue.

@szaimenszaimen removed this from the Nextcloud 26 milestone Mar 2, 2023
@szaimenszaimen added this to the Nextcloud 27 milestone Mar 2, 2023
@szaimen
szaimen merged commit a5191ce into masterMar 2, 2023
@szaimen
szaimen deleted the enh/noid/fix-right-click-images branch March 2, 2023 16:24
@szaimenszaimen modified the milestones: Nextcloud 27, Nextcloud 26Mar 2, 2023
@blizzzblizzz mentioned this pull request Mar 7, 2023
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to reviewWaiting for reviewsbug

Projects

None yet

3 participants

@szaimen@rotdrop@skjnldsv