Skip to content
This repository has been archived by the owner on Jun 17, 2023. It is now read-only.

make D3 website responsive and more accessible #27

Closed
wants to merge 5 commits into from

Conversation

keikoro
Copy link

@keikoro keikoro commented Dec 7, 2021

Hi Mike, I adapted the D3 website to make it better readable on (small) mobile devices – currently, it can only be viewed and navigated by pinching and zooming repeatedly.

Quick summary of the most important changes:

  • visually, the refactored version very closely resembles the current one (looks, in fact, almost identical for the most part)
  • I moved all inline styles out of tags and into the <style> block at the top
  • added missing HTML5 elements, replaced generic elements with better-suited HTML5 ones (<main>, <nav>, <section>,...)
  • replaced stylings/classes with fixed pixel widths for arranging content – e.g. main content block vs. right sidebar – with flex items. Result on small screens:
    • short sidebar sections get folded under their corresponding sections/paragraphs in the main content block
    • the long first sidebar section gets removed; to not lose the "social" links, I re-added them in the footer (for all screen sizes, for accessibility reasons)
    • the navigation at the top gets pulled under the logo on very small screens because it looked awkward the other way around
  • slightly reworded some link text or text surrounding links (for better accessibility when navigating links directly)

I did not look at/into the embedded D3 code, but left CSS classes that it seems to build on (e.g. for colour-formatting code snippets) untouched.

One thing that works a little differently on small screens is the "banner" with examples: I made it a lot narrower, which meant I had to drop its shadow effect, (correction: looks like I eventually figured that one out? Uh, must have forgotten about it because I initially worked on this a couple weeks ago) and because there is no hover on mobile devices which would move it around, I added scrollbars as a workaround so more of the examples can still be navigated.

@keikoro
Copy link
Author

keikoro commented Dec 7, 2021

Some screenshots:

desktop view
before
desktop_before

desktop view
after
desktop_after

new mobile view – iPhone 12/13
top of page
iPhone12-13_after_top

new mobile view – iPhone 12/13
bottom of page
iPhone12-13_after_bottom

@curran
Copy link

curran commented Dec 7, 2021

This is amazing! Thank you so much for this contribution.

@curran
Copy link

curran commented Dec 7, 2021

Did a quick QA locally and the improvements look fantastic!

image

@keikoro
Copy link
Author

keikoro commented Dec 8, 2021

Thanks, @curran!

I hadn't actually planned on this, I'd originally just wanted to get up to speed with D3, but made the "mistake" of browsing the website on my phone... ahem. Got frustrated enough I thought I'd try to fix it (for the benefit of all).

Copy link

@curran curran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@keikoro
Copy link
Author

keikoro commented Dec 10, 2021

I think I'd like to amend something before this gets merged!

I went for the last technique described on this W3C page on ARIA landmarks to label sections, which is to use the title attribute.

But, I seem to remember having read somewhere else that title isn't supported equally by screenreaders, so it might be better to use one of the other techniques, aria-label or aria-labelledby, instead (I'd go for the latter and use the headlines as label references) – if anything. Because... aria-labels should also not be overdone, and I'm wondering if in this case, they aren't necessary at all because the sections are very short to begin with and connect to one another (i.e. they aren't about completely different things, content-wise). I'm trying to find the source that support those last two arguments.

@keikoro
Copy link
Author

keikoro commented Dec 10, 2021

Hmm, well, ok, the other sources I was thinking of are several years old already, so might not be applicable anymore. However, I think I'd still change the attribute just to be on the safe side. Also, for users who don't use screenreaders, it's not actually necessary to have section titles show on hover because the sections are brief, i.e. people won't get lost in them and the effect might be more confusing/distracting than helpful.

@curran
Copy link

curran commented Dec 10, 2021

IMO the changes so far are a great improvement as they are. Subsequent iterations could be follow-on PRs.

@keikoro
Copy link
Author

keikoro commented Dec 10, 2021

@curran Ok, I'll leave it as-is then. I actually discovered a few other issues with accessibility, which requires moving some elements around – which I'm already looking into, in fact, but which might be easier to review when done separately.

I've been holding off on updating any of the links to examples for a similar reason (code vs. content).

@keikoro
Copy link
Author

keikoro commented Dec 11, 2021

Here's a recent blog post on section labelling which confirms my comments regarding that it'd be better to rework their attributes. Posting this here for reference (for a follow-up PR).

@curran
Copy link

curran commented Dec 11, 2021

Nice! Maybe go ahead and do it here if you want to. It might save some rework later. My earlier comment was just to say that this is a great improvement and mergeable as it is. But hey, if it can be even better, maybe go for it! Thanks for your contribution.

@keikoro
Copy link
Author

keikoro commented Dec 16, 2021

@curran I refactored what needed fixing – adjusted some aria attributes, reshuffled the top part of the page (landmark header, landmark navigation, D3 examples) + converted the rest of the colour notations for consistency (after making sure they worked). I also closed all paragraph tags.

I rearranged the commits because I kept running into conflicts when rebasing but kept a backup of the original commit order in my repo at bakk_make_responsive. A comparison of that with commit e606441 above shows what got refactored. (Both branches were updated to include Mike's own last changes to the repo.)

@curran
Copy link

curran commented Dec 17, 2021

Very nice! I hope someone on the D3 team can review and merge this. It looks great to me!

Copy link
Member

@Fil Fil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A great improvement!

Some changes seem unnecessary (like colors going from hex to rgb representation, or added spaces, closing tags).

I really like the row of icons at the footer, and I think we could have them in desktop mode too. (But this would be for another iteration.)

@Fil Fil requested a review from mbostock December 18, 2021 09:42
@keikoro
Copy link
Author

keikoro commented Dec 20, 2021

Some changes seem unnecessary (like colors going from hex to rgb representation, or added spaces, closing tags).

It really depends on what you want to focus on. From an end user's or the rendering computer's point of view, these changes are completely irrelevant. For a fellow developer, the state code is in – with regard to cleanliness, consistency, following conventions – can make an enormous difference; in open source, I'd argue, it can be a deciding factor (whether or not someone wants to contribute their resources to a project to begin with).

Which formatting/notation/convention code or markup should follow exactly (if any) is subjective, of course, and in projects like these ultimately up to the creators/maintainers/core devs etc. I didn't put these changes in separate commits by accident, but because I didn't want the PR to get flat-out rejected. The way they are, I can more easily revert them if they are unwanted.

IMHO, the index.html file is somewhat messy and a pain to work with either way. Its readability could easily be improved n-fold by indenting the tags/structuring them hierarchically and by shortening all those super long lines. My original final commit, before creating this PR, had done exactly that. I didn't include it so as not to "rock the boat" too much/in anticipation of reactions of this kind.

I really like the row of icons at the footer, and I think we could have them in desktop mode too. (But this would be for another iteration.)

As said in my first post, the icons at the bottom should be present in desktop mode as well!

I re-added them in the footer (for all screen sizes, for accessibility reasons)

I originally only wanted them to serve as replacement for the first "side bar", which I couldn't fit into the page nicely on small screens. But because I ended up using them to aria label the side bar links (to avoid too much duplication), they had to remain visible in the desktop view.

@keikoro
Copy link
Author

keikoro commented Dec 20, 2021

Anway, thanks for approving the changes from your side, @Fil.

The most important ones are indeed the usability improvements for end users.

@keikoro
Copy link
Author

keikoro commented Jan 7, 2022

Does it make sense to rebase this PR whenever there's an update to master? Or had I better wait until it's approved?

@Fil
Copy link
Member

Fil commented Jan 7, 2022

No need to rebase, thanks! We're just short on time atm.

@keikoro
Copy link
Author

keikoro commented Apr 3, 2022

Hi, it's been three months since the last comment on the issue, and four since I initially opened it, and it's a bit of a pity the D3 website continues to be barely usable on mobile. Is there any sort of timeframe when this will be looked at?

@curran
Copy link

curran commented Apr 6, 2022

I'd merge it and redeploy if I could! This is blocked on @mbostock 's review unfortunately.

@keikoro
Copy link
Author

keikoro commented Aug 23, 2022

@mbostock Could you give this a look-over so this can (hopefully) be merged?

I understand if this doesn't have high priority because it's not about D3 code itself, but the website is one of the first ways via which people will learn (more) about D3, so would think that accessiblity of this meta resource is also important (... and it's been ~9 months now since I opened the PR).

Alternatively, perhaps the task could be delegated?

@keikoro
Copy link
Author

keikoro commented Oct 12, 2022

Hi, I've enabled GitHub Pages on my fork so my changes can be viewed live.

It takes literally one click (on this very link):
https://keikoro.github.io/d3.github.com/

Convenience link to the D3 website so it's only one other click to compare the two side by side:
https://d3js.org/

Can be accessed from wherever, instantly. @mbostock

@curran
Copy link

curran commented Oct 14, 2022

Very nice!

@mbostock
Copy link
Member

mbostock commented Jun 7, 2023

Sorry I let this languish. This was a nice suggestion and I appreciate the contribution even though I failed to merge it. I’ve wanted for years to overhaul the D3 website completely, and therefore I was not very motivated to make further incremental improvements. We are finally making progress on the new website now with d3/d3#3654. Again, apologies for taking so long—I’ve been trying to do a lot of other things at the same time.

@mbostock mbostock closed this Jun 7, 2023
@RichMorin
Copy link

RichMorin commented Jun 7, 2023 via email

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
5 participants