-
Notifications
You must be signed in to change notification settings - Fork 44
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(web): move installation issues to a drawer and keep Install button always visible #1778
Conversation
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.
a125c5c
to
619ba5b
Compare
// 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."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn 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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks much better now! I have a few suggestions about the wording.
Co-authored-by: Imobach González Sosa <[email protected]>
Pull Request Test Coverage Report for Build 12026002261Details
💛 - Coveralls |
## Problem During the presentation of the [new location for the pre-installation checks](#1778) at a team meeting on December 9th, it was discovered that the drawer doesn't close when users click on the section they are currently viewing. ## Solution As a quick fix, the onClose callback is triggered when users click on section names. This addresses the issue in a simple way until having time to consider other behavior options, such as not rendering the current section as a link in the drawer but using different elements to hint users they are already on that section. ## Testing - Added a new unit test - Tested manually
Update to release version 11. * #1495 * #1564 * #1617 * #1618 * #1625 * #1626 * #1627 * #1628 * #1630 * #1631 * #1632 * #1633 * #1634 * #1635 * #1636 * #1639 * #1640 * #1641 * #1642 * #1643 * #1644 * #1645 * #1646 * #1647 * #1648 * #1649 * #1650 * #1651 * #1652 * #1654 * #1655 * #1656 * #1657 * #1660 * #1663 * #1666 * #1667 * #1668 * #1670 * #1671 * #1673 * #1674 * #1675 * #1676 * #1677 * #1681 * #1682 * #1683 * #1684 * #1687 * #1688 * #1689 * #1690 * #1691 * #1692 * #1693 * #1694 * #1695 * #1696 * #1698 * #1699 * #1702 * #1703 * #1704 * #1705 * #1707 * #1708 * #1709 * #1710 * #1711 * #1712 * #1713 * #1714 * #1715 * #1716 * #1717 * #1718 * #1720 * #1721 * #1722 * #1723 * #1727 * #1728 * #1729 * #1731 * #1732 * #1733 * #1734 * #1735 * #1736 * #1737 * #1740 * #1741 * #1743 * #1744 * #1745 * #1746 * #1751 * #1753 * #1754 * #1755 * #1757 * #1762 * #1763 * #1764 * #1765 * #1766 * #1767 * #1769 * #1771 * #1772 * #1773 * #1774 * #1777 * #1778 * #1785 * #1786 * #1787 * #1788 * #1789 * #1790 * #1791 * #1792 * #1793 * #1794 * #1795 * #1796 * #1797 * #1798 * #1799 * #1800 * #1802 * #1803 * #1804 * #1805 * #1807 * #1808 * #1809 * #1810 * #1811 * #1812 * #1814 * #1815 * #1821 * #1822 * #1823 * #1824 * #1825 * #1826 * #1827 * #1828 * #1830 * #1831 * #1832 * #1833 * #1834 * #1835 * #1836 * #1837 * #1838 * #1839 * #1840 * #1841 * #1842 * #1843 * #1844 * #1845 * #1847 * #1848 * #1849 * #1850 * #1851 * #1854 * #1855 * #1856 * #1857 * #1860 * #1861 * #1863 * #1864 * #1865 * #1866 * #1867 * #1871 * #1872 * #1873 * #1875 * #1876 * #1877 * #1878 * #1880 * #1881 * #1882 * #1883 * #1884 * #1885 * #1886 * #1888 * #1889 * #1890
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:
Solution
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
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:
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
Note for reviewers
Please, do a manual test too.
Screenshots
Valid installation setup
Not valid installation setup
Related to not yet planned Trello card, https://trello.com/c/DmUOQN1Y (internal link)