feat: remove username from header #551

mubbsharanwar · 2024-02-13T08:41:01Z

In the spirit of making registration experience quick and easy (more details on MVP scope doc), we want to be able to remove username from the registration form. This creates the need to remove it everywhere on the UX too. As part of this initiative, we will be removing username from the headers and making the dropdown consistent with edx.org site.

Changes
Set the flag HIDE_USERNAME_FROM_HEADER to hide the username from the header and replace the username with the name for the screen reader text when the flag is enabled.
The flag will be removed when this PR would be merged

Screenshots

Component	Before	After
Header
Studio Header
Learning Header

Ticket
VAN-1804

codecov-commenter · 2024-02-13T09:01:37Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (011b25d) 71.94% compared to head (11981e5) 72.32%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #551      +/-   ##
==========================================
+ Coverage   71.94%   72.32%   +0.37%     
==========================================
  Files          51       51              
  Lines         802      813      +11     
  Branches      156      163       +7     
==========================================
+ Hits          577      588      +11     
  Misses        215      215              
  Partials       10       10

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

syedsajjadkazmii

Would this work on the MFEs that are on the openedx organization? like frontend-app-learner-dashboard?

src/learning-header/LearningHeader.test.jsx

src/studio-header/UserMenu.jsx

src/learning-header/AuthenticatedUserDropdown.jsx

src/studio-header/UserMenu.jsx

mubbsharanwar · 2024-02-14T06:12:00Z

Would this work on the MFEs that are on the openedx organization? like frontend-app-learner-dashboard?

We set the npm alias to indicate from where the header component would be installed. For now, aliase is not added for frontend-app-learner-dashboard but we can add it to install frontend-component-header-edx.
Reference to add npm alias :https://github.com/edx/edx-internal/blob/master/frontends/frontend-app-learner-record/prod_config.yml#L6

syedsajjadkazmii · 2024-02-14T06:15:11Z

Would this work on the MFEs that are on the openedx organization? like frontend-app-learner-dashboard?

We set the npm alias to indicate from where the header component would be installed. For now, aliase is not added for frontend-app-learner-dashboard but we can add it to install frontend-component-header-edx. Reference to add npm alias :https://github.com/edx/edx-internal/blob/master/frontends/frontend-app-learner-record/prod_config.yml#L6

frontend-app-learner-dashboard was just an example, we would need to do this for all MFEs residing in openedX repo.

mubbsharanwar · 2024-02-14T06:20:37Z

Would this work on the MFEs that are on the openedx organization? like frontend-app-learner-dashboard?

We set the npm alias to indicate from where the header component would be installed. For now, aliase is not added for frontend-app-learner-dashboard but we can add it to install frontend-component-header-edx. Reference to add npm alias :https://github.com/edx/edx-internal/blob/master/frontends/frontend-app-learner-record/prod_config.yml#L6

frontend-app-learner-dashboard was just an example, we would need to do this for all MFEs residing in openedX repo.

Yes we can install in openedX repos but we need to add alias in edx-internal, for most of MFEs alias is added, you can check the detail here.
And I already shared the reference with you how to add alias.

syedsajjadkazmii

Thankyou

syedsajjadkazmii · 2024-02-16T10:16:33Z

src/Header.jsx

@@ -32,6 +32,9 @@ subscribe(APP_CONFIG_INITIALIZED, () => {
    MINIMAL_HEADER: !!process.env.MINIMAL_HEADER,
    ENTERPRISE_LEARNER_PORTAL_HOSTNAME: process.env.ENTERPRISE_LEARNER_PORTAL_HOSTNAME,
    AUTHN_MINIMAL_HEADER: !!process.env.AUTHN_MINIMAL_HEADER,
+    // this flag is introduced to unblock for new release.
+    // the code behind flag would be removed in follow up PR


the code would be removed or the flag would be removed?

flag. comment updated

syedsajjadkazmii · 2024-02-16T10:19:00Z

src/learning-header/LearningHeader.test.jsx

+    const userName = queryByTestId('username');
+    expect(userName).not.toBeInTheDocument();


Suggested change

const userName = queryByTestId('username');

expect(userName).not.toBeInTheDocument();

const username = queryByTestId('username');

expect(username).not.toBeInTheDocument();

syedsajjadkazmii · 2024-02-16T10:19:26Z

src/studio-header/StudioHeader.test.jsx

+      const userMenue = container.querySelector('#user-dropdown-menu');
+      expect(userMenue.textContent).toContain('');


Suggested change

const userMenue = container.querySelector('#user-dropdown-menu');

expect(userMenue.textContent).toContain('');

const userMenu = container.querySelector('#user-dropdown-menu');

expect(userMenu.textContent).toContain('');

abdullahwaheed · 2024-02-16T12:15:51Z

src/DesktopHeader.jsx

          data-hj-suppress
        >
-          {username}
+          {!hideUsername && username}


shouldn't it be

Suggested change

{!hideUsername && username}

{usernameOrName}

No, we don't want to show username or name when HIDE_USERNAME_FROM_HEADER is enabled.

abdullahwaheed · 2024-02-16T13:43:25Z

src/learning-header/AuthenticatedUserDropdown.jsx

 };

 AuthenticatedUserDropdown.defaultProps = {
  enterpriseLearnerPortalLink: '',
+  name: null,


should be string

Suggested change

name: null,

name: '',

Remove username from header and add name for screen reader VAN-1804

mubbsharanwar force-pushed the manwar/VAN-1804 branch 2 times, most recently from 3a42e81 to 2399197 Compare February 13, 2024 08:59

syedsajjadkazmii reviewed Feb 13, 2024

View reviewed changes

src/learning-header/LearningHeader.test.jsx Outdated Show resolved Hide resolved

src/studio-header/UserMenu.jsx Outdated Show resolved Hide resolved

src/studio-header/UserMenu.jsx Show resolved Hide resolved

syedsajjadkazmii reviewed Feb 14, 2024

View reviewed changes

src/learning-header/AuthenticatedUserDropdown.jsx Show resolved Hide resolved

src/studio-header/UserMenu.jsx Outdated Show resolved Hide resolved

syedsajjadkazmii approved these changes Feb 14, 2024

View reviewed changes

BilalQamar95 approved these changes Feb 15, 2024

View reviewed changes

abdullahwaheed approved these changes Feb 15, 2024

View reviewed changes

mubbsharanwar force-pushed the manwar/VAN-1804 branch from 12a9436 to fba5dfe Compare February 16, 2024 09:25

syedsajjadkazmii approved these changes Feb 16, 2024

View reviewed changes

mubbsharanwar force-pushed the manwar/VAN-1804 branch from fba5dfe to 38885c2 Compare February 16, 2024 12:05

abdullahwaheed reviewed Feb 16, 2024

View reviewed changes

mubbsharanwar force-pushed the manwar/VAN-1804 branch 2 times, most recently from beef0db to 4c22911 Compare February 19, 2024 07:50

abdullahwaheed approved these changes Feb 19, 2024

View reviewed changes

mubbsharanwar force-pushed the manwar/VAN-1804 branch from 4c22911 to 11981e5 Compare February 19, 2024 07:53

feat: remove username from header

b9959a3

Remove username from header and add name for screen reader VAN-1804

mubbsharanwar force-pushed the manwar/VAN-1804 branch from 11981e5 to b9959a3 Compare February 19, 2024 07:57

mubbsharanwar merged commit 1e9af3c into master Feb 19, 2024
6 checks passed

mubbsharanwar deleted the manwar/VAN-1804 branch February 19, 2024 08:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: remove username from header #551

feat: remove username from header #551

mubbsharanwar commented Feb 13, 2024 •

edited

Loading

codecov-commenter commented Feb 13, 2024 •

edited

Loading

syedsajjadkazmii left a comment •

edited

Loading

mubbsharanwar commented Feb 14, 2024

syedsajjadkazmii commented Feb 14, 2024

mubbsharanwar commented Feb 14, 2024 •

edited

Loading

syedsajjadkazmii left a comment

syedsajjadkazmii Feb 16, 2024

mubbsharanwar Feb 16, 2024

syedsajjadkazmii Feb 16, 2024

syedsajjadkazmii Feb 16, 2024

abdullahwaheed Feb 16, 2024

mubbsharanwar Feb 19, 2024

abdullahwaheed Feb 16, 2024

		const userName = queryByTestId('username');
		expect(userName).not.toBeInTheDocument();

		const userMenue = container.querySelector('#user-dropdown-menu');
		expect(userMenue.textContent).toContain('');

feat: remove username from header #551

feat: remove username from header #551

Conversation

mubbsharanwar commented Feb 13, 2024 • edited Loading

codecov-commenter commented Feb 13, 2024 • edited Loading

Codecov Report

syedsajjadkazmii left a comment • edited Loading

Choose a reason for hiding this comment

mubbsharanwar commented Feb 14, 2024

syedsajjadkazmii commented Feb 14, 2024

mubbsharanwar commented Feb 14, 2024 • edited Loading

syedsajjadkazmii left a comment

Choose a reason for hiding this comment

syedsajjadkazmii Feb 16, 2024

Choose a reason for hiding this comment

mubbsharanwar Feb 16, 2024

Choose a reason for hiding this comment

syedsajjadkazmii Feb 16, 2024

Choose a reason for hiding this comment

syedsajjadkazmii Feb 16, 2024

Choose a reason for hiding this comment

abdullahwaheed Feb 16, 2024

Choose a reason for hiding this comment

mubbsharanwar Feb 19, 2024

Choose a reason for hiding this comment

abdullahwaheed Feb 16, 2024

Choose a reason for hiding this comment

mubbsharanwar commented Feb 13, 2024 •

edited

Loading

codecov-commenter commented Feb 13, 2024 •

edited

Loading

syedsajjadkazmii left a comment •

edited

Loading

mubbsharanwar commented Feb 14, 2024 •

edited

Loading