[`admin_style_v3`] As an enterprise user i can see the back office with new brand colours #10949

jibees · 2023-06-07T08:36:25Z

What? Why?

Closes [BUU] As an enterprise user, I can see the back office with new brand colours #10630

TODO:

remove icons from primary navigation
change to Sentence case
change colours and add shadows to containers (both primary and secondary nav)
add underline to active page (both primary and secondary nav)

What should we test?

As an admin, activate admin_style_v3
As a shop manager, see that you can see the new style for the menu
As an admin, desactivate admin_style_v3
As a shop manager, see that you cannot see the new style, and everything should be equivalent to master

Release notes

Changelog Category: User facing changes

The title of the pull request will be included in the release notes.

Dependencies

Documentation updates

dacook

🤩 Great work, it's beautiful!

I submitted a new commit based on my comment, how does that look to you?

PS great commit history too, makes it super easy to follow.

dacook · 2023-06-08T00:33:07Z

app/webpacker/css/admin_v3/components/navigation.scss

+      text-transform: lowercase;
+      &::first-letter {
+        text-transform: uppercase;
+      }


Neat trick! From what I can see, this is necessary to convert "Order Cycles" to "Order cycles" and the same for "Bulk Order Management".
Why not just update the title source itself?

Hmm that took a few mins to find, I didn't realise it was quite so complicated. But still this looks like a better place to update: https://github.com/openfoodfoundation/openfoodnetwork/blob/master/app/helpers/spree/admin/navigation_helper.rb#L27-L29

I think we can use a different string helper other than .titleize

dacook · 2023-06-08T00:35:40Z

app/webpacker/css/admin_v3/components/navigation.scss

@@ -61,6 +61,8 @@ nav.menu {
 }

 #admin-menu {
+  box-shadow: 0px 1px 0px rgba(0, 0, 0, 0.05), 0px 2px 2px rgba(0, 0, 0, 0.07);


Do you think we could keep borders in variables.scss? This would help us keep them consistent where possible.

(but maybe not right now so we avoid merge conflicts!)

app/helpers/spree/admin/navigation_helper.rb

mkllnk

All good except the failing specs now.

app/helpers/spree/admin/navigation_helper.rb

mkllnk · 2023-06-08T05:13:56Z

app/webpacker/css/admin_v3/globals/palette.scss

+$color-1: #ffffff !default; // White
+$color-2: #9fc820 !default; // Green
+$color-3: #008397 !default; // Teal (Allports)
+$color-4: #6788a2 !default; // Dark Blue
+$color-5: #c85136 !default; // Red/Orange (Mojo)
+$color-6: #ff9300 !default; // Yellow
+$color-7: #eff1f2 !default; // Light grey
+$color-8: #191c1d !default; // Near-black
+$color-9: #2e3132 !default; // Dark Grey


Maybe it's difficult in the transition between styles but I find it hard to read $color-7 in CSS without knowing which one it is.

I agree, I think I'll suggest an update in a future PR.

// Probably $teal: #008397; // Allports // or $palette-teal: #008397; // Allports // and purely for backwards-compatibility during transition: $color-3: $palette-teal !default;

PS It's only now that I learnt what the !default modifier is for. It's like the ||= ruby operator, it assigns only if not already set.

I'm not sure if we ever make use of this, and it doesn't seem like a good idea, so I might suggest removing that too..

dacook

Fixed up my spec errors and applied Maikel's suggestion.

dacook · 2023-06-08T05:54:08Z

app/webpacker/css/admin_v3/globals/palette.scss

+$color-1: #ffffff !default; // White
+$color-2: #9fc820 !default; // Green
+$color-3: #008397 !default; // Teal (Allports)
+$color-4: #6788a2 !default; // Dark Blue
+$color-5: #c85136 !default; // Red/Orange (Mojo)
+$color-6: #ff9300 !default; // Yellow
+$color-7: #eff1f2 !default; // Light grey
+$color-8: #191c1d !default; // Near-black
+$color-9: #2e3132 !default; // Dark Grey


I agree, I think I'll suggest an update in a future PR.

// Probably $teal: #008397; // Allports // or $palette-teal: #008397; // Allports // and purely for backwards-compatibility during transition: $color-3: $palette-teal !default;

dacook · 2023-06-08T05:54:28Z

app/webpacker/css/admin_v3/globals/palette.scss

+$color-1: #ffffff !default; // White
+$color-2: #9fc820 !default; // Green
+$color-3: #008397 !default; // Teal (Allports)
+$color-4: #6788a2 !default; // Dark Blue
+$color-5: #c85136 !default; // Red/Orange (Mojo)
+$color-6: #ff9300 !default; // Yellow
+$color-7: #eff1f2 !default; // Light grey
+$color-8: #191c1d !default; // Near-black
+$color-9: #2e3132 !default; // Dark Grey


PS It's only now that I learnt what the !default modifier is for. It's like the ||= ruby operator, it assigns only if not already set.

I'm not sure if we ever make use of this, and it doesn't seem like a good idea, so I might suggest removing that too..

dacook · 2023-06-08T05:55:05Z

app/webpacker/css/admin_v3/components/navigation.scss

@@ -61,6 +61,8 @@ nav.menu {
 }

 #admin-menu {
+  box-shadow: 0px 1px 0px rgba(0, 0, 0, 0.05), 0px 2px 2px rgba(0, 0, 0, 0.07);


(but maybe not right now so we avoid merge conflicts!)

Co-Authored-By: Maikel <maikel@email.org.au>

+ prettier

CSS property `capitalize` uppercase all first letter of each words. I need to use a small trick with `::first-letter` selector

+ add some padding to separate those borders then

+ prettier

I chose .capitalize instead of .humanize, because the latter has extra features which may not be desired.

jibees · 2023-06-08T09:25:37Z

Reading all your suggestions/comments/.. I think we're ready now, right?

drummer83 · 2023-06-09T19:23:44Z

Hi @jibees,
I am assuming that this is ONLY covering the main navigation (primary and secondary) at the top of the screen, correct? The content of the pages is still showing old colors here and there. Let me know if it is already time to report the old colors which still need to be adjusted.

For the main navigation I can confirm that

the new colors are used when the feature toggle is active.
the old colors are used when the feature toggle is not active.
the primary as well as the secondary navigation are showing the new colors.

Here is an example where I think we would want to have more capital letters than just the first one. Shouldn't it be OIDC settings or OIDC Settings?

Here is an example where I found some tiny bits of old colors, but I assume this is not the time to report those now:

Conclusion

It's looking great! 🎉
I think we can merge and I will open an issue for the OIDC settings navigation. ➡️ #10982
Thanks again! 🚀

jibees · 2023-06-12T07:11:22Z

I am assuming that this is ONLY covering the main navigation (primary and secondary) at the top of the screen, correct?

Right.

Here is an example where I think we would want to have more capital letters than just the first one. Shouldn't it be OIDC settings or OIDC Settings?

I'd vote for OIDC Settings and not Oidc_settings :-)

Thanks!

jibees self-assigned this Jun 7, 2023

jibees force-pushed the 10630-buu-as-an-enterprise-user-i-can-see-the-back-office-with-new-brand-colours branch 2 times, most recently from 6a5c158 to 60b9dfd Compare June 7, 2023 12:39

jibees changed the title ~~As an enterprise user i can see the back office with new brand colours~~ [admin_style_v3] As an enterprise user i can see the back office with new brand colours Jun 7, 2023

jibees force-pushed the 10630-buu-as-an-enterprise-user-i-can-see-the-back-office-with-new-brand-colours branch from f95aa78 to fc428c3 Compare June 7, 2023 14:28

jibees requested review from dacook and mariocarabotta June 7, 2023 14:29

jibees marked this pull request as ready for review June 7, 2023 14:29

dacook approved these changes Jun 8, 2023

View reviewed changes

mkllnk reviewed Jun 8, 2023

View reviewed changes

app/helpers/spree/admin/navigation_helper.rb Show resolved Hide resolved

mkllnk reviewed Jun 8, 2023

View reviewed changes

dacook force-pushed the 10630-buu-as-an-enterprise-user-i-can-see-the-back-office-with-new-brand-colours branch from 22a77cf to 458859d Compare June 8, 2023 06:00

dacook reviewed Jun 8, 2023

View reviewed changes

jibees and others added 17 commits June 8, 2023 11:24

AdminV3: Remove icon from icon from primary navigation menu

e34f4a9

Co-Authored-By: Maikel <maikel@email.org.au>

Copy/Paste file admin/components/navigation under admin_v3 folder

eab46b6

+ prettier

Change case to Sentence case

5ee3550

CSS property `capitalize` uppercase all first letter of each words. I need to use a small trick with `::first-letter` selector

Prettier file

dd48092

Add new color: $color-9 #2E3132 Dark Grey

a4d3011

Remove background-color on menu

b163d4a

No more white on labels but dark grey

ccd7466

Adjust font size for labels in menu

d51786a

Adjust color to orange when selected or hover

ba98039

Add new color: $color-7 #EFF1F2 Light grey

2057449

Add border with box-shadow to menu and sub-menu

1b42366

Add border bottom when hovering or selected

51f47fc

+ add some padding to separate those borders then

Copy/Paste file admin/shared/layout under admin_v3 folder

b865d8c

+ prettier

Add new color: $color-8 #191c1d Near-black

905220a

Remove background, border and adjust margin top

d7f7ff7

Adjust color to the title of the page

d6cb483

Set sentence case tab names in Ruby instead of CSS

e2ba18f

I chose .capitalize instead of .humanize, because the latter has extra features which may not be desired.

jibees force-pushed the 10630-buu-as-an-enterprise-user-i-can-see-the-back-office-with-new-brand-colours branch from f11f4e4 to e2ba18f Compare June 8, 2023 09:24

mkllnk approved these changes Jun 8, 2023

View reviewed changes

drummer83 added the pr-staged-fr staging.coopcircuits.fr label Jun 9, 2023

drummer83 self-assigned this Jun 9, 2023

drummer83 merged commit 13037d2 into openfoodfoundation:master Jun 9, 2023
51 checks passed

drummer83 removed the pr-staged-fr staging.coopcircuits.fr label Jun 9, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[`admin_style_v3`] As an enterprise user i can see the back office with new brand colours #10949

[`admin_style_v3`] As an enterprise user i can see the back office with new brand colours #10949

jibees commented Jun 7, 2023 •

edited

dacook left a comment •

edited

dacook Jun 8, 2023

dacook Jun 8, 2023

dacook Jun 8, 2023

mkllnk left a comment

mkllnk Jun 8, 2023

dacook Jun 8, 2023

dacook Jun 8, 2023

dacook left a comment

dacook Jun 8, 2023

dacook Jun 8, 2023

dacook Jun 8, 2023

jibees commented Jun 8, 2023

drummer83 commented Jun 9, 2023

jibees commented Jun 12, 2023

[admin_style_v3] As an enterprise user i can see the back office with new brand colours #10949

[admin_style_v3] As an enterprise user i can see the back office with new brand colours #10949

Conversation

jibees commented Jun 7, 2023 • edited

What? Why?

What should we test?

Release notes

Dependencies

Documentation updates

dacook left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mkllnk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dacook left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jibees commented Jun 8, 2023

drummer83 commented Jun 9, 2023

Conclusion

jibees commented Jun 12, 2023

[`admin_style_v3`] As an enterprise user i can see the back office with new brand colours #10949

[`admin_style_v3`] As an enterprise user i can see the back office with new brand colours #10949

jibees commented Jun 7, 2023 •

edited

dacook left a comment •

edited