Skip to content

Add css variable support and use MD icon #1178

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
nickvergessen merged 3 commits into from
Apr 26, 2022
Merged

Add css variable support and use MD icon #1178

nickvergessen merged 3 commits into from
Apr 26, 2022

Conversation

@skjnldsv
Copy link
Member

@skjnldsv skjnldsv commented Mar 31, 2022 •
edited
Loading

  1. Migrate the icon to material design
  2. Switch to css variable compatibility
  3. Use EmptyContent
  4. Use getCapabilities
  5. use css to animate

Instant change on theming update:
Peek 31-03-2022 18-27

image

TODO in follow-up:

  • Migrate the scss file to inner vue css file
  • Cleanup old styling and vars usage

@skjnldsv skjnldsv added this to the Nextcloud 25 milestone Mar 31, 2022
@skjnldsv skjnldsv requested a review from nickvergessen March 31, 2022 16:26
@skjnldsv skjnldsv self-assigned this Mar 31, 2022
nickvergessen
nickvergessen reviewed Mar 31, 2022
View reviewed changes
nickvergessen
nickvergessen reviewed Mar 31, 2022
View reviewed changes
@skjnldsv skjnldsv force-pushed the feat/primary-css-variable branch from a9ff908 to 333cc60 Compare April 1, 2022 07:41
@nickvergessen nickvergessen force-pushed the feat/primary-css-variable branch from 333cc60 to b56229c Compare April 1, 2022 09:27
@nickvergessen

This comment was marked as resolved.

Sign in to view
@nickvergessen

This comment was marked as resolved.

Sign in to view
@skjnldsv

This comment was marked as resolved.

Sign in to view
@nickvergessen

This comment was marked as resolved.

Sign in to view
@skjnldsv

This comment was marked as resolved.

Sign in to view
@blizzz blizzz mentioned this pull request Apr 7, 2022
Merged
@skjnldsv skjnldsv modified the milestones: Nextcloud 24, Nextcloud 25 Apr 7, 2022
skjnldsv and others added 2 commits April 22, 2022 15:57
@skjnldsv
Add css variable support and use MD icon
467e9f3 
Signed-off-by: John Molakvoæ (skjnldsv) <[email protected]>
@nickvergessen @skjnldsv
Recompile
6f9f9b5 
Signed-off-by: Joas Schilling <[email protected]>
@skjnldsv skjnldsv requested a review from nickvergessen April 22, 2022 13:57
@skjnldsv skjnldsv force-pushed the feat/primary-css-variable branch from b56229c to 6f9f9b5 Compare April 22, 2022 13:57
@skjnldsv skjnldsv requested a review from szaimen April 22, 2022 13:57
@skjnldsv
Copy link
Member Author

skjnldsv commented Apr 22, 2022

Done @nickvergessen

@skjnldsv skjnldsv mentioned this pull request Apr 22, 2022
Closed
46 tasks
szaimen
szaimen previously requested changes Apr 22, 2022
View reviewed changes
Copy link
Contributor

@szaimen szaimen left a comment •
edited by skjnldsv
Loading

Choose a reason for hiding this comment

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

Seems to work but didn't review the code.

Please adjust

notifications/css/styles.scss

Line 63 in 6f9f9b5

max-height: calc(100vh - 50px * 4 - 44px) !important;
to max-height: calc(100vh - 50px * 4 - 80px) !important;

in order to fix image

@skjnldsv skjnldsv requested a review from szaimen April 22, 2022 15:46
@skjnldsv
Copy link
Member Author

skjnldsv commented Apr 22, 2022

Fixed and fixed the animation too
Peek 22-04-2022 17-46

@skjnldsv
Copy link
Member Author

skjnldsv commented Apr 22, 2022

/compile amend /

@skjnldsv @nextcloud-command
Fix scrolling and animation
91a019f 
Signed-off-by: John Molakvoæ <[email protected]>
Signed-off-by: nextcloud-command <[email protected]>
@nextcloud-command nextcloud-command force-pushed the feat/primary-css-variable branch from b011100 to 91a019f Compare April 22, 2022 15:49
szaimen
szaimen approved these changes Apr 25, 2022
View reviewed changes
Copy link
Contributor

@szaimen szaimen left a comment

Choose a reason for hiding this comment

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

Looks good from the screencast

@nickvergessen nickvergessen merged commit 6d8da15 into master Apr 26, 2022
@nickvergessen nickvergessen deleted the feat/primary-css-variable branch April 26, 2022 06:34
danxuliu added a commit to nextcloud/server that referenced this pull request May 3, 2022
@danxuliu
Adjust acceptance tests to changes in notifications markup
3c451f8 
Since nextcloud/notifications#1178 each notification in the list is a
"<li>" rather than a "<div>".

Signed-off-by: Daniel Calviño Sánchez <[email protected]>
@danxuliu danxuliu mentioned this pull request May 3, 2022
Merged
@skjnldsv skjnldsv mentioned this pull request Aug 11, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nickvergessen nickvergessen nickvergessen approved these changes

@szaimen szaimen szaimen approved these changes

Assignees

@skjnldsv skjnldsv

Labels

3. to review design technical debt

Projects

None yet

Milestone

Nextcloud 25

Development

Successfully merging this pull request may close these issues.

4 participants

@skjnldsv @nickvergessen @szaimen