Happy Hacktoberfest, everyone! Here's a breakdown of some of the Hacktoberfest-eligible bugs in the Forem codebase. We'd love to see you tackle these in October. 🎃 Frontend
After merging github.com/forem/forem/pull/14635, we have a lot of prop-types errors in the console that need to be cleaned up. These errors can be observed throughout the onboarding flow and during general app usage when running the app locally.
Each error will need to be investigated to check whether the component.propTypes declaration needs to be changed, or whether the prop being passed to the given component needs to be changed.
We can/should complete this work in multiple small PRs (e.g. per page).
Describe the bug
On my computer, when I'm in a post with a sidebar on the right which shows User profile and More articles👉 I can not scroll the sidebar to view more.
To Reproduce
Go to a post
Navigate to the right sidebar
Try to scroll
Can not scroll
Expected behavior
The sidebar should scroll for showing more info.
Desktop (please complete the following information):
OS, version: Windows 10 | macOS Big Sur
Browser, version: Microsoft Edge 94.0.992.31 (Official build) (64-bit) | Safari 15.0 (16612.1.29.41.4, 16612) | Google Chrome 93.0.4577.82 (Official Build) (arm64)
Additional context
The sidebar is fully shown in narrow width or on mobile.
When navigating to the Forem hamburger menu and then scrolling down, you cannot scroll back up. The menu gets stuck and instead when you try to scroll up it scrolls the page underneath the menu. The only way to scroll up and down sucessfully is to press and hold as you scroll on the screen.
To Reproduce
Open hamburger menu
Scroll all the way down
Try to scroll back up
Expected behavior
Scrolling up and down on the menu should work smoothly and independently of the page underneath the menu.
The left-side navigation options on the dashboard page (desktop view) are correctly wrapped in a <nav> element but they should also be wrapped in a <ul>, each as individual <li> list items.
This helps screen reader users navigate between the items, knowing how many options are in the list and their current position in that list.
To Reproduce
Using a screen reader visit the left-hand navigation on the /dashboard page
Notice the number of items in the navigation is not announced
Expected behavior
Items are wrapped in a list and the number of items is announced when using a screen reader
On the user dashboard there are two "banner" landmarks:
The <header> at the top of the page (with logo, search, etc)
The <header> containing "Total post reactions", "Total post views" etc
The same issue is observable across the other dashboards also (e.g dashboard/following_tags).
On each dashboard, we should only have one banner landmark on the page. While the wrapper for the "total post reactions" etc can just be changed to a div, this content should also be moved to live inside the <main> element, and this change might need a little bit of CSS tweaking to make sure everything continues to look consistent.
To Reproduce
Log in to the app
Go to /dashboard
Open up the dev tools and run the axe extension
Notice the error "Document should not have more than one banner landmark"
Expected behavior
The error should not appear in the axe report
Visually everything should remain the same
The same should apply to the other dashboard pages
Screenshots
Additional context
Each dashboard page could be tackled in separate PRs if this keeps things more manageable.
There are a few places in the app where we show a "hamburger" menu to open more details. For example:
Left side of the header when in a mobile viewport
Left and right side of a tag page (e.g.dev.to/t/javascript) when in tablet viewport or smaller
These menus currently lack the ARIA attributes to convey they are popup views, and the focus management required to reach and operate them properly by keyboard. For example, if you open the menu with the keyboard, then keep pressing tab, you'll notice that your keyboard focus continues to scroll behind the menu content and it's actually very hard to reach any of the links in the menu.
Although these views are primarily visible when using a touch device, they need to be keyboard accessible because:
Some users will utilise keyboards, switch controls, or other assistive technology with a touch device
Screen reader users rely on keyboard focus being in the correct place when dialogs open/close, even when swipe gestures are used on mobile devices
Desktop users may see these layouts if zooming in to view the page more clearly
To Reproduce
Using your devtools, or by resizing/zooming the window, shrink the viewport so that the hamburger menu is visible
Using the keyboard, Tab to the hamburger menu button and press Enter
Keep tabbing and notice it's really hard to reach the menu content and you're scrolling behind the menu
Expected behavior
I think the most straightforward approach will be to treat these menus like our dropdowns. We should be able to re-use our initializeDropdown helper function to get all these behaviors without much additional code, but in summary:
The "hamburger button" has the ARIA attributes aria-haspopup, aria-controls and aria-expanded, so that screen reader users know that it activates some dynamic content
When open aria-expanded is "true"
When closed, aria-expanded is "false"
When the menu opens, focus is sent to the first link inside it
When open, the Escape key closes the menu
When closed either by Escape or by clicking the 'X' button, focus is returned to the "hamburger button"
If the user Tabs past the last link in the menu, the menu closes
In this case, the content could reasonably exist within <aside> elements ( "complementary" landmark), since they support the main content, but are not essential to it, and make sense when separated from the main content.
NB: This refers to the large-screen 'desktop' view pictured above only. On smaller screens, the content is triggered as a pop-up menu. For the purposes of this issue we do not wish to change any of the markup for the pop-up menu.
To Reproduce
Visit a tag's page
In devtools, use the axe extension to check the page
Notice the error "All page content should be contained by landmarks"
Expected behavior
The error should not appear in an axe report
When I navigate by landmarks using a screen reader, I should be able to select a landmark to navigate to this sidebar content
Additional context
There is a different class of accessibility issues associated with the smaller screen 'menu popup' view for this content. It will be captured in a separate issue, and needn't affect the fix here.
The issue with the above logic is that a new article which should become part of that endpoint will not flush its cache if created. So it will not show up on that endpoint until another article which fits within the above @articles is purged. This makes it kind of random to find out when it will be included.
article.purge is a magic method to create a purge ping which uses the appropriate surrogate header.
A solution for this could be a surrogate key like this...
and then we make a specific purge call when a new article should purge it like purge("tag-index-endpoint-#{tag}" when an article is created.
Our current situation isn't causing any radically wrong behavior and the "random" purging is working okay but we should look into this as we go about bigger changes here.
Each tag has an associated taggings_count which contains the number of items they are associated with (in our case either articles or listings).
The problem lies in the fact that some of these counts are incorrect in the DB.
I found this out by playing with the Tags API which returns tags sorted by taggings_count in descending order
For example:
archlinux according to the API - dev.to/api/tags - is the third most used tag but it's very unlikely that is true. Its tag page - dev.to/t/archlinux - lists 50 articles and even taking into account all listings it might have been used in, it doesn't add up
dev.to/api/tags?per_page=15 shows that devops is more popular than showdev but devops page contains 2917 articles, showdev page contains 3110. Again, I don't think there are enough listings tagged with devops to account for the difference
Our app has really nice pleasant web pull-to-refresh functionality on most pages (not all, we don't want this behavior on some pages like chat, for example).
But the way it works, the user needs to be stopped at the top of the page and then initiate a pull up before it works like a charm.
I've hacked away at this a bit so I know it's possible to fix with some logical adjustments to how the whole thing works, but I think it needs a finer solution than whatever I'd hack together quickly.
Occasionally, when viewing the /latest feed, users can see more recently published posts appearing below older posts. For example:
Initially I thought this was an issue coming from the API, but I think it might actually be to do with how the frontend constructs and inserts paged-in articles in the feed. In the example screengrab above, the network responses shows that the "Tracking responses..." article should have appeared in a different part of the sequence (the surrounding articles pictured above were not returned from the same network fetch).
TL;DR - it needs more investigation, but I think the source of the issue may be in how the paging behaviour plays with the chronological feed.
To Reproduce
Go to the /latest feed
Start scrolling and keep an eye on the published at date on the feed cards
Eventually you'll see some that appear out of chronological order
Expected behavior
The articles in the "latest" feed should appear in the order they were published at, with the newest first.
When navigating around Dev.to the my tags list section of the side bar occasionally will not populate. Refreshing the page seems to always fix this. Sometimes the is a console error at the same time there is a failure.
To Reproduce
Go to the dev.to home page and sign in.
Change from feed to week and watch the my tags section
also try cycling through the different feed types (week/month/year/infinity/latest) and note that every other click results in no "My Tags" list
Intermittently you can go to an article and navigate back to the homepage with the "DEV" Logo and also see the "My Tags" list not present
Expected behavior
"My Tags" list should always populate
Screenshots
Desktop):
OS, version: MacOS 10.15.5
Browser, version: Chrome 84
Additional context
I tried this both in and out of incognito mode. The only difference seemed to be that incognito mode did not have the console errors
Happy Hacktoberfest, everyone! Here's a breakdown of some of the Hacktoberfest-eligible bugs in the Forem codebase. We'd love to see you tackle these in October. 🎃
Frontend