feat(web): move installation issues to a drawer and keep Install button always visible #1778

dgdavid · 2024-11-21T16:38:23Z

Problem

A month ago , the 'Install' button achieved more visibility after being moved to the sticky header, allowing users to access it from any page/section/screen.

But that was just an intermediate step towards improving the 'Install' action and the discoverability of 'Installation setup problems,' which sadly introduced some drawbacks:

The Install button is displayed dynamically, which still forces users to figure out where it will appear when it’s not visible from the start.
When installation is not possible, users have to navigate to the Overview page to see what is making their setup invalid.

Solution

Keep the install button always enabled and visible, triggering the installation when the setup is valid or displaying found issues when not, and
Allow users to check found problems always, not only in the overview

A discarded alternative

At commit 6ecdceb, this set of changes was in a bit different shape, where the interface rendered a disabled "Install" button and an enabled "Warning" button to display the setup problems. Additionally, it featured a tooltip to explain to users why the "Install" button was disabled and where to click to view the reasons in detail.

Click to show/hide a few screenshots

Install button disabled	Install button tooltip

Preflight checks visible	Install button enabled

However, this approach created unnecessary complexity and moving pieces, not to mention the challenge of writing a clear, concise, yet complete explanation for the tooltip and keeping it up-to-date, including translations

What's more, there is no reason to avoid keeping the "Install" button always active but behaving slightly different depending on the setup status:

Displaying found problems when the installation setup is invalid
Displaying the confirmation dialog when the setup is valid

This way, the user doesn't need to think. They can simply click the "Install" button. If it's not possible, they can immediately start fixing their setup. If it is possible, they just have to confirm they want to begin the installation.

Testing

Added a new unit test
Tested manually

Note for reviewers

Please, do a manual test too.

Screenshots

Valid installation setup

Install button	onClick: Confirmation dialog

Not valid installation setup

Install button	onClick: Preflight checks

Related to not yet planned Trello card, https://trello.com/c/DmUOQN1Y (internal link)

But disabled when the installation is not possible because of settings issues.

Avoiding to use the term "issues" in the title because it isn't an accurate term. "Preflight checks" has been used instead.

To avoid "stacking contexts" problems which avoid sticky page sections behave as expected. Such a workaround will be not needed when migrating to latest versions of PF6, since the problem has been addressed by droping the DrawerContentBody from the Page component, see patternfly/patternfly#7130 and related links See also https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_positioned_layout/Understanding_z-index/Stacking_context

Replaced by IssuesDrawer and IssuesDrawerToggle.

Instead of doing it dinamically since the IssuesDrawer already have logic for rendering nothing when needed. This approach helps to avoid having calls to a query when the QueryClient has not been set yet, like at login screen.

Which was also causing stacking problems at the selection product page.

Because it provides a benefit to the user both visually and cognitively, as they don't have to figure out why the install button was disabled, nor be told where to find the reasons for it.

dgdavid · 2024-11-25T08:20:44Z

web/src/components/core/InstallButton.tsx

+  // TRANSLATORS: The install button label
+  const buttonText = _("Install");
+  // TRANSLATORS: Accessible text included with the install button when there are issues
+  const withIssuesAriaLabel = _("Not possible with current setup. Click to know more.");


Reviewers, note that this is part of the install button. Thus, an screen reader will announce "Install not possible with...". Please, have the screen reader in mind when proposing improvements.

Kooha-2024-11-25-08-23-04.webm

BTW, I'd like to explore moving it to an aria-description or so, but still having to learn a bit more about the screen reader interaction.

dgdavid · 2024-11-25T08:26:50Z

web/src/components/core/IssuesDrawer.tsx

+
+  return (
+    <NotificationDrawer ref={ref}>
+      <NotificationDrawerHeader title={_("Preflight checks")} onClose={onClose} />


Reviewers, please note the use of "Preflight checks" to avoid the "Issues" technicality. A first attempt was to use "Installation issues", but these are not actually "Installation issues" since the installation does not even started. It is more an "Installation settings issues", but sounds difficult to explain. Looks like "Preflight checks" could be a more self-explanatory term. As usual, better ideas are more than welcome.

"Preflight checks" means something to me since I used flight sims a lot 1995-2005, but I am not sure if a normal user can make sense of this; much less one with limited English knowledge.

I don't even dare to think about translations and how much they make sense to users.

Please let's not get overly creative with this; keep it simple.

@shundhammer

What is your proposal?

How about "consistency checks"?

Try to avoid lingo from a completely different area of technology like aviation.

Not sure about the word "consistency". Sounds weird to me in this context. We could go for "Installation checks" or "Settings checks", but both are too generic IMHO

dgdavid · 2024-11-25T08:28:14Z

web/src/components/core/IssuesDrawer.tsx

+            {_(
+              "Current installation settings does not meet the necessary requirements for installing the selected product. Click on each section name to access the relevant page and adjust the settings as needed.",


Reviewers, feel free to improve this text as much as you want. But please resist the temptation to remove it. UI explanations are an aid, not a problem.

Regardless of the wording, please fix the broken English:

"Current installation settings don't meet the necessary requirements..."

("settings": Plural -> s/"does not"/"do not"/ which nobody ever says (or writes outside of legal documents), so "don't")

How about something along the lines of

"You need to make some decisions about some of the settings.
Click on each section name to check and maybe change the setting."

Don't make it sound like it's a problem of the installer; it's not.

Address the user directly to clarify that it needs user interaction.

Keep it simple. Keep not-so-fluent English speakers in mind.

Don't fall into the "let's resort to legalese BS" trap:
First of all, the message needs to be easy to understand, not so much to cover all eventualities.

"You need to make some decisions about some of the settings.
Click on each section name to check and maybe change the setting."

I like some bits of your proposal, specially these that helps to simplify the text but keeping all the information the user need to understand that some actions still necessary, thanks!

What about

Before install, you still having to take some decisions about some of the settings. Click on each section name to provide required setting

As you said above, I'd like keep some consistency between the user action and the message shown. I mean,

User clicks on "Install"
The installer shows something that does not install
But the installer also tell the user "Before install, X"

So, user can be sure that this dialog has been for sure shown because as a side effect of clicking on the Install action, not an unexpected message.

"Before install" is broken English. It's either "Before installing" or "Before the installation" (or maybe "Before starting the installation". Also, it's "make decisions", not "take decisions".

But your last proposal is much better than the initial text.

"Before install" is broken English. It's either "Before installing"

I mean "Before installing", of course. I ~~do not~~ don't want to break English more that it is already :P

imobachgs

It looks much better now! I have a few suggestions about the wording.

web/src/assets/styles/patternfly-overrides.scss

web/src/components/core/InstallButton.test.tsx

web/src/components/core/InstallButton.tsx

web/src/components/core/IssuesDrawer.tsx

web/src/components/layout/Layout.tsx

Co-authored-by: Imobach González Sosa <[email protected]>

coveralls · 2024-11-26T07:46:14Z

Pull Request Test Coverage Report for Build 12026002261

Details

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 71.063%

Totals
Change from base Build 12013658798:	0.0%
Covered Lines:	16925
Relevant Lines:	23817

💛 - Coveralls

dgdavid added 10 commits November 21, 2024 12:35

feature(web): Add IssuesDrawer components

35b78c4

feature(web) Allow Install button be always visible

37088af

But disabled when the installation is not possible because of settings issues.

feature(web) Use an application drawer to show issues

2393ed0

Avoiding to use the term "issues" in the title because it isn't an accurate term. "Preflight checks" has been used instead.

feature(web): stop rendering issues at overview page

9cbebd2

fix(web): drop no longer necessary IssuesLink

67ca130

Replaced by IssuesDrawer and IssuesDrawerToggle.

fix(web): mount the issues drawer always

1833db9

Instead of doing it dinamically since the IssuesDrawer already have logic for rendering nothing when needed. This approach helps to avoid having calls to a query when the QueryClient has not been set yet, like at login screen.

fix(web): drop a not necessary z-index

32474aa

Which was also causing stacking problems at the selection product page.

fix(web): change text of the disabled Install button

6ecdceb

feature(web): unify Install and IssuesDrawerToggle buttons

619ba5b

Because it provides a benefit to the user both visually and cognitively, as they don't have to figure out why the install button was disabled, nor be told where to find the reasons for it.

dgdavid force-pushed the always-display-install-button branch from a125c5c to 619ba5b Compare November 22, 2024 13:58

dgdavid marked this pull request as ready for review November 25, 2024 08:11

dgdavid commented Nov 25, 2024

View reviewed changes

imobachgs reviewed Nov 25, 2024

View reviewed changes

dgdavid and others added 3 commits November 25, 2024 12:00

doc(web): update changelog

fdb90d3

Apply wording suggestions from code review

0b1e0db

Co-authored-by: Imobach González Sosa <[email protected]>

Merge branch 'master' into always-display-install-button

67898ea

dgdavid requested a review from imobachgs November 26, 2024 20:47

imobachgs approved these changes Nov 26, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(web): move installation issues to a drawer and keep Install button always visible #1778

feat(web): move installation issues to a drawer and keep Install button always visible #1778

dgdavid commented Nov 21, 2024 •

edited

Loading

dgdavid Nov 25, 2024 •

edited

Loading

dgdavid Nov 25, 2024

dgdavid Nov 25, 2024 •

edited

Loading

shundhammer Nov 25, 2024

dgdavid Nov 25, 2024 •

edited

Loading

shundhammer Nov 25, 2024

dgdavid Nov 25, 2024

dgdavid Nov 25, 2024

shundhammer Nov 25, 2024 •

edited

Loading

shundhammer Nov 25, 2024

dgdavid Nov 25, 2024

dgdavid Nov 25, 2024

shundhammer Nov 25, 2024 •

edited

Loading

dgdavid Nov 25, 2024 •

edited

Loading

imobachgs left a comment •

edited

Loading

coveralls commented Nov 26, 2024 •

edited

Loading

		{_(
		"Current installation settings does not meet the necessary requirements for installing the selected product. Click on each section name to access the relevant page and adjust the settings as needed.",

feat(web): move installation issues to a drawer and keep Install button always visible #1778

Are you sure you want to change the base?

feat(web): move installation issues to a drawer and keep Install button always visible #1778

Conversation

dgdavid commented Nov 21, 2024 • edited Loading

Problem

Solution

A discarded alternative

Testing

Note for reviewers

Screenshots

Valid installation setup

Not valid installation setup

dgdavid Nov 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dgdavid Nov 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dgdavid Nov 25, 2024 • 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

shundhammer Nov 25, 2024 • 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

shundhammer Nov 25, 2024 • edited Loading

Choose a reason for hiding this comment

dgdavid Nov 25, 2024 • edited Loading

Choose a reason for hiding this comment

imobachgs left a comment • edited Loading

Choose a reason for hiding this comment

coveralls commented Nov 26, 2024 • edited Loading

Pull Request Test Coverage Report for Build 12026002261

Details

💛 - Coveralls

dgdavid commented Nov 21, 2024 •

edited

Loading

dgdavid Nov 25, 2024 •

edited

Loading

dgdavid Nov 25, 2024 •

edited

Loading

dgdavid Nov 25, 2024 •

edited

Loading

shundhammer Nov 25, 2024 •

edited

Loading

shundhammer Nov 25, 2024 •

edited

Loading

dgdavid Nov 25, 2024 •

edited

Loading

imobachgs left a comment •

edited

Loading

coveralls commented Nov 26, 2024 •

edited

Loading