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 sharing panel lighthouse warnings #33321

Merged
merged 2 commits into from
Jul 25, 2022

Conversation

PVince81
Copy link
Member

Replaced h5 with span and adjusted selectors.
Fixed ul/li hierarchy of some elements.

Found by running the Lighthouse chromium tool in snapshot mode while having the share panel open AND the link share dropdown

Replaced h5 with span and adjusted selectors.
Fixed ul/li hierarchy of some elements.

Signed-off-by: Vincent Petry <[email protected]>
@PVince81
Copy link
Member Author

I tried using the exact same versions of node and npm like CI and there's no difference.
Also in parallel I had a PR to document the versions to use: #33322

@PVince81
Copy link
Member Author

PVince81 commented Jul 22, 2022

  • next attempt: try with a clean env

(and yes I can summon the bot but now I'm really tired that every PR doesn't match expectations)

@PVince81
Copy link
Member Author

even with a fresh clone I get the same result as the previous commit...

@PVince81
Copy link
Member Author

ah, so it's the icons: dist/icons.css

I thought we added a step to build it as part of "make build-js-production"

@PVince81
Copy link
Member Author

alright... need to finish #32753

@PVince81
Copy link
Member Author

/compile amend /

node v14.20.0
npm 8.15.0

Signed-off-by: Vincent Petry <[email protected]>
Signed-off-by: nextcloud-command <[email protected]>
@nextcloud-command nextcloud-command force-pushed the bugfix/noid/fix-sharing-lighthouse-issues branch from 9ac7a95 to b7ae739 Compare July 22, 2022 17:17
@PVince81 PVince81 merged commit 0a956e2 into master Jul 25, 2022
@PVince81 PVince81 deleted the bugfix/noid/fix-sharing-lighthouse-issues branch July 25, 2022 09:41
@skjnldsv skjnldsv mentioned this pull request Aug 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants