feat(headerbar): display instance's & app's info in profile menu - LIBS-176

[Mohammer5]

Blocked by #820
Relates to LIBS-176 (link)

Open ToDos

• Cypress test

Open questions

• When the instance's version is too long, the build version breaks into the next line.
  With the current styles it's a lot harder to parse
• I have to load the instance's data from the server, so there's a short loading time (the app data is available though). What should we display during that time?
• If an error occurs while loading the instance's info, right now I just don't render anything. Is that correct or should we display some kind of error message?
• Right now there's no proper way of retrieving the app's build revision, so I just left it out. Is that okay? Or is that information crucial?

[dhis2-bot]

🚀 Deployed on https://pr-795--dhis2-ui.netlify.app

[cypress]

---

Test summary

556 • 1 • 0 • 0

---

Run details

Project	ui
Status	Failed
Commit	5237b5c
Started	Nov 23, 2021 10:41 AM
Ended	Nov 23, 2021 10:50 AM
Duration	09:11 💡
OS	Linux Ubuntu - 20.04
Browser	Electron 89

View run in Cypress Dashboard ➡️

---

Failures

	components/header-bar/src/features/the_headerbar_contains_a_profile_menu.feature		1
1	The HeaderBar contains a profile menu > The HeaderBar shows an image icon if the user has an avatar

---

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

[cooper-joe]

When the instance's version is too long, the build version breaks into the next line.

Tweaking the font and spacing here improves this, I think. Using line-height: 17px; and margin-bottom: 6px produces the below, which I think is easier to parse as two separate items.

I have to load the instance's data from the server, so there's a short loading time (the app data is available though). What should we display during that time?

I think Checking DHIS2 version... makes sense to describe what's happening. If possible, using grey500 for the color while it's loading makes it even clearer I think.

If an error occurs while loading the instance's info, right now I just don't render anything. Is that correct or should we display some kind of error message?

I think it warrants an error, just to the user doesn't assume it's still pending. Using the text There was a problem getting DHIS2 version.. I think it's OK to use the regular text style, it doesn't warrant a critical alert style.

Right now there's no proper way of retrieving the app's build revision, so I just left it out. Is that okay? Or is that information crucial?

I'm not sure about this one. I included it in the mockup as it was mentioned in the ticket, but I'm not sure if it's crucial. @amcgee - do you know if that info is important?

[varl]

I think leaving out the build revision is OK for now.

[Mohammer5]

Draft: Needs a cypress test

[amcgee]

@Mohammer5 @cooper-joe looks good, a few points:

1. The version has a hover style but isn't clickable, I think it probably shouldn't have a hover style?
2. There shouldn't be a comma between the app name and the version (as per spec)
3. @cooper-joe we use "DHIS2 version X.Y.Z" but not "Data Visualizer version x.y.z" (version is omitted), is that intentional?

[amcgee]

See above

[Mohammer5]

I addressed the first and second point. Waiting for @cooper-joe's response to the third one

[cooper-joe]

@cooper-joe we use "DHIS2 version X.Y.Z" but not "Data Visualizer version x.y.z" (version is omitted), is that intentional?

Yes, this was intentional, though I can see how it's inconsistent. Reasons were:

• having version after DHIS2 makes it easier to seperate the 2 from the version number (e.g. DHIS2 2.34 is harder to read than DHIS2 version 2.34, especially at small sizes)
• omitting version after the app name makes the label shorter, which in turn makes this label easier to read at small sizes.

You're right that this is slightly odd though, and the inconsistency might cause confusion. I think omitting version completely avoids this (and avoids more i18n work).

[Mohammer5]

I've removed the word "version".
Once #820 has been merged, I'll address the cypress issues and re-request reviews

[Mohammer5]

Everything's been addressed & I covered the code with cypress.
Requesting another review from @amcgee

[varl]

@cooper-joe @amcgee @Mohammer5

I think we should avoid hard coding "DHIS2" for the same reason we have custom logo support. E.g. DATIM may want to show its own version string and product name, instead of DHIS2 and its version.

[stale]

Hi!

Due to a lack of activity on this issue over time (180 days) it seems to be stale. If still relevant, please provide information that moves it forward, e.g. additional information, a pull request with suggested changes, or a reason to keep it open.

Any activity will keep it open, otherwise it will be closed automatically in 30 days. Thanks! 🤖

[Mohammer5]

Still relevant, just went under the radar

[dhis2-bot]

🚀 Deployed on https://pr-795--dhis2-ui.netlify.app

[amcgee]

I'm reviving this and adding PWA update notification, with slightly updated styling and using context instead of loading the version directly. So I'll close this PR and have opened #1153 - but it's based on your work @Mohammer5 so big hats off in thanks to you!