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 vue material icons #1816

Merged
merged 11 commits into from
Apr 14, 2021
Merged

Fix vue material icons #1816

merged 11 commits into from
Apr 14, 2021

Conversation

skjnldsv
Copy link
Contributor

@skjnldsv skjnldsv commented Apr 1, 2021

  • Move to md icons
    • Replace
    • Sizes and design match checks
    • Regression checks
  • Cleanup old iconfont
  • Cleanup webpack config

For all that were possible, I switched to the material design icons straight away.
For the others (like pure css without slots or our custom statuses), I moved back to a simple url-loader and a theme--dark selector.

It will clean the already-too-complicated webpack config, as well as reducing the size of the final build. I took the liberty of fixing some blatant typos and changed a few docs.

The icons used are not 16px, because MaterialDesign ships 24px with margins.
So we're on 24px now ⚠️


🖼️ Screenshots from this doc where everything is changed (Click to expand)

Capture d’écran_2021-04-01_17-58-21
Capture d’écran_2021-04-01_18-12-54
Capture d’écran_2021-04-01_18-13-26
Capture d’écran_2021-04-01_18-13-38
Capture d’écran_2021-04-01_18-15-34
Capture d’écran_2021-04-01_18-16-06
Capture d’écran_2021-04-01_18-17-26
Capture d’écran_2021-04-01_18-18-28
Capture d’écran_2021-04-01_18-18-57
Capture d’écran_2021-04-01_18-19-13
Capture d’écran_2021-04-01_18-19-30
Capture d’écran_2021-04-01_18-19-36
Capture d’écran_2021-04-01_18-19-44
Capture d’écran_2021-04-01_18-21-48
Capture d’écran_2021-04-01_18-22-11

Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
@skjnldsv skjnldsv force-pushed the enh/no-iconfonts branch 2 times, most recently from 67c9228 to 82ac3fb Compare April 1, 2021 15:43
@skjnldsv skjnldsv self-assigned this Apr 1, 2021
@skjnldsv skjnldsv added 2. developing Work in progress enhancement New feature or request technical debt labels Apr 1, 2021
@skjnldsv skjnldsv added this to the 4.0.0 milestone Apr 1, 2021
@skjnldsv skjnldsv added 3. to review Waiting for reviews design Design, UX, interface and interaction design and removed 2. developing Work in progress labels Apr 1, 2021
@skjnldsv skjnldsv marked this pull request as ready for review April 1, 2021 16:25
Copy link
Contributor

@jotoeri jotoeri left a comment

Choose a reason for hiding this comment

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

Yeah! The icons just disappeared on the bundles... AppNavigation falls from 50 KiB to 20 KiB 😅 Nice step! 🎉
Honestly i didn't check all the css in detail, but generally looks good to me... ☺️

src/components/SettingsSection/SettingsSection.vue Outdated Show resolved Hide resolved
src/components/AppNavigationItem/InputConfirmCancel.vue Outdated Show resolved Hide resolved
src/components/AppNavigationToggle/AppNavigationToggle.vue Outdated Show resolved Hide resolved
src/components/Content/Content.vue Outdated Show resolved Hide resolved
src/components/ColorPicker/ColorPicker.vue Outdated Show resolved Hide resolved
src/components/SettingsSection/SettingsSection.vue Outdated Show resolved Hide resolved
skjnldsv added 6 commits April 2, 2021 10:35
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
@skjnldsv skjnldsv requested a review from jotoeri April 2, 2021 08:45
Copy link
Contributor

@jotoeri jotoeri left a comment

Choose a reason for hiding this comment

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

One thing - but apart of that. 👍 🎉

src/components/AppNavigationItem/InputConfirmCancel.vue Outdated Show resolved Hide resolved
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
@raimund-schluessler
Copy link
Contributor

raimund-schluessler commented Apr 2, 2021

There is (literally) a small issue with the last arrow in the breadcrumbs bar. The longer the text of the last crumb is, the smaller the arrow gets:

Screenshot_2021-04-02 Nextcloud Vue Style Guide

Besides of that, it seems work well and reduced the bundle size of my app by 250 kB. 👍

Copy link
Contributor

@JonathanTreffler JonathanTreffler left a comment

Choose a reason for hiding this comment

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

Reducing the bundles size is great and necessary. Seems good to me :)

Copy link
Contributor

@raimund-schluessler raimund-schluessler left a comment

Choose a reason for hiding this comment

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

Co-authored-by: Raimund Schlüßler <raimund.schluessler@mailbox.org>
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
@skjnldsv
Copy link
Contributor Author

skjnldsv commented Apr 3, 2021

There is (literally) a small issue with the last arrow in the breadcrumbs bar. The longer the text of the last crumb is, the smaller the arrow gets:

Screenshot_2021-04-02 Nextcloud Vue Style Guide

Besides of that, it seems work well and reduced the bundle size of my app by 250 kB.

@raimund-schluessler
Weirdly, this was not present in the documentation.
So I assume there is a conflicting rule on server?

https://60672dfeacb72f0007de3ed6--nextcloud-vue-components.netlify.app/

@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 Apr 3, 2021
Copy link
Contributor Author

@skjnldsv skjnldsv left a comment

Choose a reason for hiding this comment

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

We need to release v3.9.0 before merging

@skjnldsv skjnldsv added the breaking PR that requires a new major version label Apr 3, 2021
@raimund-schluessler
Copy link
Contributor

There is (literally) a small issue with the last arrow in the breadcrumbs bar. The longer the text of the last crumb is, the smaller the arrow gets:
Screenshot_2021-04-02 Nextcloud Vue Style Guide
Besides of that, it seems work well and reduced the bundle size of my app by 250 kB.

@raimund-schluessler
Weirdly, this was not present in the documentation.
So I assume there is a conflicting rule on server?

https://60672dfeacb72f0007de3ed6--nextcloud-vue-components.netlify.app/

This screenshot is from the netlify docs. But you need to change the title of the last crumb to something longer. The default one is to short to trigger the issue.

Copy link
Contributor

@juliusknorr juliusknorr left a comment

Choose a reason for hiding this comment

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

Very nice 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish breaking PR that requires a new major version design Design, UX, interface and interaction design enhancement New feature or request technical debt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants