SAK-51017 Portal skin fixes and design improvements #13427

st-manu · 2025-03-10T12:30:33Z

https://sakaiproject.atlassian.net/browse/SAK-51017

kunaljaykam · 2025-03-10T13:19:00Z

library/src/skins/default/src/sass/modules/tool/_menu.scss

-			&.current,
-			a.current {


I added a.current becuase of gradebook toolnavbar structure. so we need it or we can change the gradebook code.

sorry, I hadn't seen the recent SAK-51050

kunaljaykam · 2025-03-10T13:32:43Z

library/src/skins/default/src/sass/modules/tool/portal/_portal.scss

  grid-area: header;
  background-color: var(--top-header-background);
+  border-bottom: 3px solid var(--sakai-border-color);
+  /*box-shadow: rgba(0, 0, 0, 0.07) 0px 2px 4px, rgba(0, 0, 0, 0.07) 0px 4px 8px;*/ // scrollbar issue


do we need the comment?

no, initially a teammate suggested adding a shadow. I left it commented to remind myself to tell them why I removed it

kunaljaykam · 2025-03-10T13:41:35Z

library/src/skins/default/src/sass/modules/tool/portal/_portal.scss

+    &:hover::before,
+    &:focus::before {
+      content: "\f4ec";
+    }


why do we need to add this content?

I think, @st-manu can tell if I'm wrong, that this is the unselected pin icon to make it visible always and not only on mouseover

what @SedueRey says (which we’ve also done) isn’t exactly this. it’s to visually indicate that the pin will be removed. in fact, it’s the unselected pin icon

kunaljaykam · 2025-03-10T17:34:44Z

library/src/skins/default/src/sass/modules/tool/_menu.scss

+        max-height: 0;
+        overflow: hidden;


st-manu · 2025-03-11T07:30:54Z

thanks for the review @kunaljaykam

st-manu · 2025-03-11T08:07:56Z

@ottenhoff

with the changes in portal, a cypress test needs to be modified: https://github.com/sakaicontrib/cypress-sakai/blob/7097aa4157f2bfebc7ba7fb12c7533e28c4c7bc0/cypress/e2e/sakai-trunk/assignment.js#L170 .

for the mobile view, the site list access has been unified, so the button is now:

<button class="btn icon-button responsive-allsites-button"
        data-bs-toggle="offcanvas"
        data-bs-target="#portal-nav-sidebar"
        aria-controls="portal-nav-sidebar"
        aria-label="$rloader.sit_navigation"
        aria-expanded="false"
        title="$rloader.sit_navigation">
  <i class="bi bi-grid-3x3-gap-fill"></i>
</button>

I've never worked with cypress before. would replacing button.btn-sidebar-collapse with button.portal-nav-sidebar be enough?

kunaljaykam · 2025-03-11T08:37:30Z

portal/portal-render-engine-impl/impl/src/webapp/vm/morpheus/snippets/header-snippet.vm

+      <a class="btn" href="/portal" title="$rloader.portal_home">
+        <img class="logo-light" src="/library/skin/$!{pageSkin}/images/sakaiLogoBlack.png" alt="Sakai Logo">
+        <img class="logo-dark" src="/library/skin/$!{pageSkin}/images/sakaiLogo.png" alt="Sakai Logo">
+      </a>


do you think we can use js to load only one logo as per the selected theme? Its 75kb each i guess, so not massive.

I don't think it's best to have a logo hidden in the js that each institution has to adapt. but, from my side, I don't have a problem with it, whatever is considered best

But ... What if the institution needs to customize it further?
A logo for dark mode, another one for mobile view, etc etc.
I think the best thing to do is to remove the velocity logo and add it via CSS.

after merging these changes, I'll create a Jira to move the logo to the css, and we'll get started on it @kunaljaykam @SedueRey

kunaljaykam · 2025-03-11T08:40:52Z

portal/portal-render-engine-impl/impl/src/webapp/vm/morpheus/includeSiteNav.vm

+        if (currentSiteElement) {
+            // to add margin top
+            const offsetLi = document.createElement('li');
+            currentSiteElement.parentNode.insertBefore(offsetLi, currentSiteElement);
+            offsetLi.scrollIntoView({behavior: 'auto', block: 'start', inline: 'nearest'});
+        }


Please explain, why we are creating li

I just changed it because, in addition, it was discussed to always place the selected site in the first position, not just scroll to it

ottenhoff · 2025-03-12T18:18:05Z

I've never worked with cypress before. would replacing button.btn-sidebar-collapse with button.portal-nav-sidebar be enough?

Yes

ottenhoff · 2025-03-12T18:18:55Z

portal/portal-render-engine-impl/impl/src/webapp/vm/morpheus/includeAccountNav.vm

@@ -46,7 +37,7 @@ For opening help sidebar
        aria-label="$rloader.sit_allsites"
        aria-controls="select-site-sidebar"
        title="$rloader.sit_allsites">
-      <i class="si si-all-sites"></i>
+      <i class="bi bi-grid-3x3-gap-fill"></i>


Suggested change

ottenhoff · 2025-03-12T18:19:37Z

portal/portal-render-engine-impl/impl/src/webapp/vm/morpheus/includeMobileFooter.vm

+          aria-label="$rloader.sit_navigation"
+          aria-expanded="false"
+          title="$rloader.sit_navigation">
+        <i class="bi bi-grid-3x3-gap-fill"></i>


Suggested change

ottenhoff · 2025-03-12T18:19:58Z

portal/portal-render-engine-impl/impl/src/webapp/vm/morpheus/includePage.vm

+
+        #if (${currentRole} && $roleSwitchState == false)
+            <div class="current-role mb-3">
+                <i class="fa fa-user" aria-hidden="true"></i>


Suggested change

ottenhoff · 2025-03-12T18:22:26Z

portal/portal-render-engine-impl/impl/src/webapp/vm/morpheus/includeSiteNav.vm

-                <path d="M1 2a1 1 0 0 1 1-1h2a1 1 0 0 1 1 1v2a1 1 0 0 1-1 1H2a1 1 0 0 1-1-1V2zm5 0a1 1 0 0 1 1-1h2a1 1 0 0 1 1 1v2a1 1 0 0 1-1 1H7a1 1 0 0 1-1-1V2zm5 0a1 1 0 0 1 1-1h2a1 1 0 0 1 1 1v2a1 1 0 0 1-1 1h-2a1 1 0 0 1-1-1V2zM1 7a1 1 0 0 1 1-1h2a1 1 0 0 1 1 1v2a1 1 0 0 1-1 1H2a1 1 0 0 1-1-1V7zm5 0a1 1 0 0 1 1-1h2a1 1 0 0 1 1 1v2a1 1 0 0 1-1 1H7a1 1 0 0 1-1-1V7zm5 0a1 1 0 0 1 1-1h2a1 1 0 0 1 1 1v2a1 1 0 0 1-1 1h-2a1 1 0 0 1-1-1V7zM1 12a1 1 0 0 1 1-1h2a1 1 0 0 1 1 1v2a1 1 0 0 1-1 1H2a1 1 0 0 1-1-1v-2zm5 0a1 1 0 0 1 1-1h2a1 1 0 0 1 1 1v2a1 1 0 0 1-1 1H7a1 1 0 0 1-1-1v-2zm5 0a1 1 0 0 1 1-1h2a1 1 0 0 1 1 1v2a1 1 0 0 1-1 1h-2a1 1 0 0 1-1-1v-2z"></path>
-            </svg>
+            <i class="bi bi-grid-3x3-gap-fill d-none d-md-inline"></i>
+            <i class="bi bi-arrow-right-circle-fill d-inline d-md-none"></i>


Using a instead of an tag is often favored for semantic reasons: • Semantic Clarity: The tag originally denotes text that is set off in an alternate voice (often italicized), not icons. A is a neutral inline container that doesn’t imply any specific presentation, making it a cleaner semantic choice when the element doesn’t convey textual emphasis. • Accessibility: A semantically neutral element paired with appropriate ARIA roles (if needed) can improve clarity for assistive technologies, ensuring the icon’s purpose is clear. • Convention vs. Meaning: While frameworks like Font Awesome popularized the tag for icons, this is more of a historical convention than a semantic mandate. Using aligns more with the idea of marking up content based on its meaning rather than its default presentation.

ottenhoff · 2025-03-12T19:41:23Z

portal/portal-render-engine-impl/impl/src/webapp/vm/morpheus/includePortalHeaderBreadcrumb.vm

+    </div>
+    <div class="bi bi-chevron-right portal-header-breadcrumb-separator"></div>
+    #end
+    #end


Suggested change

#end

#end

st-manu force-pushed the SAK-51017 branch from c94b9a4 to bea9059 Compare March 10, 2025 12:46

kunaljaykam reviewed Mar 10, 2025

View reviewed changes

st-manu force-pushed the SAK-51017 branch from c3dfeef to 8021df1 Compare March 11, 2025 07:46

kunaljaykam reviewed Mar 11, 2025

View reviewed changes

ern changed the title ~~SAK-51017 Trinity: several fixes and improvements~~ SAK-51017 Portal skin fixes and design improvements Mar 11, 2025

st-manu and others added 5 commits March 12, 2025 12:03

SAK-51017 Trinity: several fixes and improvements

78c9d61

SAK-51017 Trinity: several fixes and improvements - menu

e60f236

hamburguer button color fix

be92737

SAK-51017 kunaljaykam review

c8497fc

menu: move the current-site to the top

220a8b3

st-manu force-pushed the SAK-51017 branch from aa00e73 to 220a8b3 Compare March 12, 2025 11:04

ottenhoff reviewed Mar 12, 2025

View reviewed changes

SAK-51017 ottenhoff review

7f3b04e

jesusmmp approved these changes Mar 13, 2025

View reviewed changes

jesusmmp merged commit 2d54f14 into sakaiproject:master Mar 13, 2025
5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SAK-51017 Portal skin fixes and design improvements #13427

SAK-51017 Portal skin fixes and design improvements #13427

st-manu commented Mar 10, 2025

kunaljaykam Mar 10, 2025 •

edited

Loading

st-manu Mar 11, 2025

kunaljaykam Mar 10, 2025

st-manu Mar 11, 2025

kunaljaykam Mar 10, 2025

SedueRey Mar 11, 2025

st-manu Mar 11, 2025

kunaljaykam Mar 10, 2025

st-manu commented Mar 11, 2025

st-manu commented Mar 11, 2025 •

edited

Loading

kunaljaykam Mar 11, 2025

st-manu Mar 11, 2025

SedueRey Mar 11, 2025

st-manu Mar 12, 2025

kunaljaykam Mar 11, 2025

st-manu Mar 11, 2025

ottenhoff commented Mar 12, 2025

ottenhoff Mar 12, 2025

ottenhoff Mar 12, 2025

ottenhoff Mar 12, 2025

ottenhoff Mar 12, 2025 •

edited

Loading

ottenhoff Mar 12, 2025

	<i class="bi bi-grid-3x3-gap-fill"></i>
	<span class="bi bi-grid-3x3-gap-fill" aria-hidden="true"></span>

	<i class="fa fa-user" aria-hidden="true"></i>
	<span class="fa fa-user" aria-hidden="true"></span>

SAK-51017 Portal skin fixes and design improvements #13427

SAK-51017 Portal skin fixes and design improvements #13427

Conversation

st-manu commented Mar 10, 2025

kunaljaykam Mar 10, 2025 • edited Loading

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

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

st-manu commented Mar 11, 2025

st-manu commented Mar 11, 2025 • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ottenhoff commented Mar 12, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ottenhoff Mar 12, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kunaljaykam Mar 10, 2025 •

edited

Loading

st-manu commented Mar 11, 2025 •

edited

Loading

ottenhoff Mar 12, 2025 •

edited

Loading