Skip to content

fix(settings): apps list layout #44236

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
skjnldsv merged 11 commits into from
Mar 19, 2024
Merged

fix(settings): apps list layout #44236

skjnldsv merged 11 commits into from
Mar 19, 2024

Conversation

@ShGKme
Copy link
Contributor

@ShGKme ShGKme commented Mar 16, 2024 •
edited
Loading

Summary

Fix Apps Settings layout:

Screenshots

Safari bug

BeforeAfter
Safari-before.mp4
Safari-after.mp4

Other screenshots

Before After
Your apps top
image image
Your apps bottom
image image
Your apps (regression - level is shown) 600px
image image
Your apps (regression - level is shown) 320px (new - actions are hidden)
image image
App bundles .
image image
Store View 1440px
image image
Store View (regression - no change with sidebar) 1440px with sidebar
image image
Store View 1280px
image image
Store View (regression - no change with sidebar) 1280px with sidebar
image image
Store View 600px
image image
Store View (regression - no change with sidebar) 600px with sidebar
image image
Store View 320px
image image
Search no results
image .
Search (invalid HTML) Your apps
image image
image image

Checklist

@ShGKme ShGKme added this to the Nextcloud 29 milestone Mar 16, 2024
@ShGKme ShGKme self-assigned this Mar 16, 2024
@Altahrim Altahrim mentioned this pull request Mar 18, 2024
Merged
@ShGKme ShGKme force-pushed the fix/settings--app-list-scrolling-on-safari branch 4 times, most recently from e4aec63 to 04af997 Compare March 18, 2024 20:45
@ShGKme ShGKme changed the title fix(settings ): apps list layout fix(settings): apps list layout Mar 18, 2024
@ShGKme ShGKme marked this pull request as ready for review March 18, 2024 20:57
@ShGKme ShGKme added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Mar 18, 2024
@ShGKme
Copy link
Contributor Author

ShGKme commented Mar 18, 2024

dron is not related

susnux
susnux reviewed Mar 19, 2024
View reviewed changes
apps/settings/src/components/AppList/AppItem.vue
</script>

<style scoped lang="scss">
@use '../../../../../core/css/variables.scss' as variables;
Copy link
Contributor

@susnux susnux Mar 19, 2024

Choose a reason for hiding this comment

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

They are deprecated it feels not right to still use them as we probably want to get rid of them soon.

Copy link
Contributor Author

@ShGKme ShGKme Mar 19, 2024

Choose a reason for hiding this comment

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

Without PostCSS, would you prefer to use just constant numbers in @media queries?

susnux
susnux approved these changes Mar 19, 2024
View reviewed changes
Copy link
Contributor

@susnux susnux left a comment

Choose a reason for hiding this comment

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

Finally the overflow issue is fixed, thank you!

@susnux susnux requested a review from artonge March 19, 2024 11:56
@susnux
Copy link
Contributor

susnux commented Mar 19, 2024

BTW I think we should in the future separate the table and grid components. That are two different designs and currently the grid one looks a bit "sad".

@ShGKme
Copy link
Contributor Author

ShGKme commented Mar 19, 2024

BTW I think we should in the future separate the table and grid components. That are two different designs and currently the grid one looks a bit "sad".

Yeah. A lot place for further refactoring here. I kept myself from not doing it, to not overload one PR with the fix by refactoring 🙈

ShGKme added 8 commits March 19, 2024 15:38
@ShGKme
refactor(settings): remove unused styles from apps settings
eac928a 
Signed-off-by: Grigorii K. Shartsev <[email protected]>
@ShGKme
refactor(settings): move apps list styles to components
d181a82 
Signed-off-by: Grigorii K. Shartsev <[email protected]>
@ShGKme
refactor(settings): use more BEM styles in apps settings
17c865d 
Signed-off-by: Grigorii K. Shartsev <[email protected]>
@ShGKme
refactor(settings): move AppItem styles to AppItem component
73dec20 
Signed-off-by: Grigorii K. Shartsev <[email protected]>
@ShGKme
fix(settings): app list scroll on Safari
a173f90 
Signed-off-by: Grigorii K. Shartsev <[email protected]>
@ShGKme
fix(settings): make apps list search result valid table
9397599 
Signed-off-by: Grigorii K. Shartsev <[email protected]>
@ShGKme
test(settings): adjust e2e tests selectors to new structure
127e361 
Signed-off-by: Grigorii K. Shartsev <[email protected]>
@ShGKme
chore(assets): Recompile assets
3ff6211 
Signed-off-by: Grigorii K. Shartsev <[email protected]>
@ShGKme ShGKme force-pushed the fix/settings--app-list-scrolling-on-safari branch from 04af997 to 3ff6211 Compare March 19, 2024 14:40
@ShGKme
Copy link
Contributor Author

ShGKme commented Mar 19, 2024

Rebased onto master

@ShGKme ShGKme enabled auto-merge March 19, 2024 14:41
@ShGKme ShGKme mentioned this pull request Mar 19, 2024
Closed
8 tasks
@skjnldsv skjnldsv disabled auto-merge March 19, 2024 15:16
@skjnldsv skjnldsv merged commit 174c10a into master Mar 19, 2024
@skjnldsv skjnldsv deleted the fix/settings--app-list-scrolling-on-safari branch March 19, 2024 15:16
@skjnldsv skjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Mar 19, 2024
@ShGKme
Copy link
Contributor Author

ShGKme commented Mar 19, 2024

/backport to stable28

@backportbot backportbot bot mentioned this pull request Mar 19, 2024
Merged
@ShGKme ShGKme mentioned this pull request Mar 19, 2024
Closed
4 tasks
@susnux susnux mentioned this pull request Mar 29, 2024
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@susnux susnux susnux approved these changes

@artonge artonge artonge approved these changes

@JuliaKirschenheuter JuliaKirschenheuter Awaiting requested review from JuliaKirschenheuter

@solracsf solracsf Awaiting requested review from solracsf

@Pytal Pytal Awaiting requested review from Pytal

@joshtrichards joshtrichards Awaiting requested review from joshtrichards

Assignees

@ShGKme ShGKme

Labels

4. to release Ready to be released and/or waiting for tests to finish bug feature: settings regression

Projects

None yet

Milestone

Nextcloud 29

Development

Successfully merging this pull request may close these issues.

[Bug]: Apps UI not working in WebKit browsers (Safari, iOS/iPadOS, GNOME/Epiphany)

5 participants

@ShGKme @susnux @artonge @skjnldsv