Skip to content
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

Fixed position chapter CTAs #2196

Merged
merged 14 commits into from Jun 15, 2021
Merged

Fixed position chapter CTAs #2196

merged 14 commits into from Jun 15, 2021

Conversation

AbbyTsai
Copy link
Contributor

@AbbyTsai AbbyTsai commented May 8, 2021

progress on Fixed position chapter CTAs #1617

Copy link
Member

@tunetheweb tunetheweb left a comment

Choose a reason for hiding this comment

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

This is excellent work @AbbyTsai - works really well!

I've made some suggestions below.

src/static/css/almanac.css Outdated Show resolved Hide resolved
src/static/css/almanac.css Outdated Show resolved Hide resolved
src/static/css/almanac.css Outdated Show resolved Hide resolved
src/templates/base/base_chapter.html Outdated Show resolved Hide resolved
src/templates/base/base_chapter.html Outdated Show resolved Hide resolved
src/templates/base/base_chapter.html Outdated Show resolved Hide resolved
src/static/css/almanac.css Outdated Show resolved Hide resolved
@AbbyTsai
Copy link
Contributor Author

Good point's mentioned. the files have been updated.
thought if we focus on CTA buttons in the full page, then as readers of footer, some of drop-button, social-media icon might be overlapped. shall we consider them behind the footer or any thoughts?

for example:
image

@tunetheweb
Copy link
Member

@HTTPArchive/designers do you have any opinions here?

I've put a test version here that you can play with: https://20210511t182132-dot-webalmanac.uk.r.appspot.com

  • Not sure if we should have the CTAs up slightly from the bottom right hand corner to avoid the footer issue? Is it possible with CSS to have them in bottom right until we reach the bottom with position sticky?
  • Also do they take over too much space on mobile? Should we not show them there? Or maybe only show the comment one?
  • Should we offer a "Hide" option?
  • Should we show all CTAs in the Explore the Results section, were they used to be as well? Might allow them still to be discovered if hidden or not shown on mobile.
    • Any other thoughts or opinions?

@AbbyTsai
Copy link
Contributor Author

Thanks. Barry. happy to see the test version and feel free to add anything below together.

Here's some consolidated points would love to have all your thoughts. @rviscomi @HTTPArchive/designers

5 CTAs (2 new, 3 alive): call to action (CTA) and hopefully be visible by readers.

CTA1:Comments (alive) CTA2:suggest edit (new) CTA3:help translate (new)
CTA4:Review result (alive) CTA5:Review queries. (alive)

Purpose: increase readers' engagement.

  • Proposed option A:
    pull CTA1-comment stick on the right bottom by scroll.
    4 CTAs retain in the existed explore result section.

  • Proposed option B:
    pull 3 CTAs1,2,3-comment/edit/help stick on the right bottom by scroll.
    2 CTAs retain in the existed explore result section.

src/static/css/almanac.css Outdated Show resolved Hide resolved
src/static/css/almanac.css Outdated Show resolved Hide resolved
@AbbyTsai
Copy link
Contributor Author

thanks all the warm, helpful recommendations.
The update 3CTAs, sticking on the right bottom by scroll and stop at end of article (before chapter-navigation/ footer), look forward to further thoughts/comments.
Also, not quiet sure what and how to fix the internal server error of PR?

@tunetheweb
Copy link
Member

Looks like GitHub is experiencing issues: https://www.githubstatus.com/incidents/zbpwygxwb3gw

Will rerun them again later.

@AbbyTsai
Copy link
Contributor Author

Thanks to the information and hope to see a lovely v here.

@tunetheweb tunetheweb added design Creating the Almanac UX development Building the Almanac tech stack labels May 17, 2021
@tunetheweb tunetheweb added this to TODO in 2021 via automation May 17, 2021
@tunetheweb tunetheweb added this to the 2021 Launch 🚀 milestone May 17, 2021
Copy link
Member

@tunetheweb tunetheweb left a comment

Choose a reason for hiding this comment

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

Love the position: sticky changes but spotted a few issues and think we can make it even smoother. Let me know what you think.

src/static/css/almanac.css Outdated Show resolved Hide resolved
src/static/css/almanac.css Outdated Show resolved Hide resolved
src/templates/base/base_chapter.html Outdated Show resolved Hide resolved
src/templates/base/base_chapter.html Outdated Show resolved Hide resolved
@AbbyTsai
Copy link
Contributor Author

Yep, cool idea. These horizonal CTAs play the role gently, but a little thought comes to the mind, would we seemingly put the same buttons both at the end of article and explore result in screen?

@tunetheweb
Copy link
Member

Yep, cool idea. These horizonal CTAs play the role gently, but a little thought comes to the mind, would we seemingly put the same buttons both at the end of article and explore result in screen?

Yeah I'm not loving that to be honest:

Duplication of CTAs at bottom of article

But I do like that they stop scrolling down at a point, rather than covering the other buttons and/or footer. I think it's cleaner than what we originally had.

As I say, the options are:

  1. Leave as is - may it's not that bad?
  2. Make the CTA buttons vanish after we reach the bottom of the article. I can't see how to do this with pure CSS so would likely need some IntersectionObserver JavaScript and not sure if that's overkill for this?
  3. Make the CTAs rest in the "Explore your Results" section so there is no duplication. I think that could be tricky to pull off and also the tab order will be incorrect as the CTAs are at the top of the article really. Also is a good progressive enhancement in case the position sticky doesn't work (e.g. reader mode).
  4. Remove the CTAs from the "Explore your Results" section so there is no duplication like you already had. I do like there there though as they are a way to "Explore the Results" and think it's a bit weird having some there and some not because they are in the scroll. Also is a good progressive enhancement in case the position sticky doesn't work (e.g. reader mode).

@HTTPArchive/designers or @HTTPArchive/developers any thoughts here? Or anyone with between CSS/JS knowledge than me that could make this work better? You can see the current implementation on this demo deploy.

@tunetheweb
Copy link
Member

Having all the options under the "Explore your Options" section also gives options to reduce the floating CTAs if we want in certain circumstances:

  • Do we only show the discuss one on mobile as limited screen space? And have all 5 floating CTAs on desktop, but only the 3 on tablet?
  • Do we allow a Close button at some point in future to remove them completely for those that find them annoying?
  • Do we allow a preference setting at some point in future to remove them completely and permanently for those that find them annoying?

It's always nice to have options 😁

@shantsis
Copy link
Contributor

It's quite a lot of actions shown by default - what if they're collapsed first?
Screen Shot 2021-05-20 at 8 36 22 PM

@AbbyTsai
Copy link
Contributor Author

AbbyTsai commented May 21, 2021

@rviscomi @tunetheweb @shantsis
Here's something would like to hear more thoughts together, as those 3 CTAs aim to touch readers' fingers in.

1.Where's CTAs position?
On the top of right side.

2.Where's CTAs scrolling area?
Inside article, from introduction to conclusion-explore result.

3.What's CTAs collapsed look?
3 svgs, the desktop's as same as mobile. (Thanks shantsis' good idea and illustration.)

4.What's in explore result?
3 CTAs+2 result buttons with text description and show 2 rows in desktop, 5 rows in mobile.

@tunetheweb
Copy link
Member

I wonder if we're trying to over use this. Do we really need "Help Translate" available at all times? And it's fairly common to have "Suggest Edit" just at the end rather than available at all times.

Yes we could hide in a menu, but a menu means we're hiding the comments again - and primary aim of this was to increase that engagement I think. We also wanted to increase interaction with the stats but the availability of those by each figure does that job IMHO.

So I'm wondering if we should leave this as just the comments? That would keep it's size to a minimum and also avoid any concerns about it covering content or repeating the "Explorer the results" section:

Test deploy with just that: https://20210522t125050-dot-webalmanac.uk.r.appspot.com

Mobile:

Mobile CTA over intro Mobile CTA over Explore the Results

Desktop:

Desktop CTA

Desktop CTA over Explore the results

@tunetheweb
Copy link
Member

Had to do some other changes so committed my additions to this branch so they wouldn't be lost. Hope you don't mind @AbbyTsai !

@AbbyTsai
Copy link
Contributor Author

Really like the lately test version where most focus on the comment CTA via discuss. Would appreciate if it's committed as well, as a little bit confused the changes.

Think that if we still wanna to raise the helper of chapter translate or make it easier to open issue via GitHub, then we might use icon of comment instead of menu and then 3 dedicated icons display by click. Would that collapsed icon attract readers' eye as well? OR we may focus on one comment CTA only. It seemingly depends on our ambition. ^^

@tunetheweb
Copy link
Member

Another sweet css:hover might has a similar effect without the dance. (demo)

It's still not keyboard accessible. Nor does it add the appropriate aria attributes necessary for screen-readers.

The thing we'd consider is the influence between mouse-over and mouse-click in desktop, but mobile.
Comparison in reader's action, then mouse-over might be an easy one to trigger, or probably mouse-click?

Another reason not to try a pure CSS solution.

Also curious how the reason you guys determine the way in development, generally, as there might be various methods (css, js, .. library etc) ?

Well that's the trick! It's a combination of what's the best solution for the users (this should be the primary concern!), what's the easiest and cleanest to code, and how maintainable it is in the future.

Adding a library is easy, but can add bloat, and add future work to maintain it as it changes. So need to be sure it's worth it.

We've (only half consciously) tried to avoid libraries for the Web Almanac site itself (though do use lots for our build process) as it's a fairly simple site at the end of the day and not needed them.

1 similar comment
@tunetheweb
Copy link
Member

Another sweet css:hover might has a similar effect without the dance. (demo)

It's still not keyboard accessible. Nor does it add the appropriate aria attributes necessary for screen-readers.

The thing we'd consider is the influence between mouse-over and mouse-click in desktop, but mobile.
Comparison in reader's action, then mouse-over might be an easy one to trigger, or probably mouse-click?

Another reason not to try a pure CSS solution.

Also curious how the reason you guys determine the way in development, generally, as there might be various methods (css, js, .. library etc) ?

Well that's the trick! It's a combination of what's the best solution for the users (this should be the primary concern!), what's the easiest and cleanest to code, and how maintainable it is in the future.

Adding a library is easy, but can add bloat, and add future work to maintain it as it changes. So need to be sure it's worth it.

We've (only half consciously) tried to avoid libraries for the Web Almanac site itself (though do use lots for our build process) as it's a fairly simple site at the end of the day and not needed them.

@AbbyTsai
Copy link
Contributor Author

Thanks Barry for the nice sharing in development principle of user first and warm sample in JS version.

Happy to see these CTAs would help our users find an easy way to get involved by the articles.

Not quiet sure if we try to add a role, aria attributes to its html element on css:mouse over, does then the screen read the missing keyboard? or?

In summary, love to see our mouse-click button alive if experience show users enjoy it, or it's a better way to keyboard accessible.

@tunetheweb
Copy link
Member

Not quiet sure if we try to add a role, aria attributes to its html element on css:mouse over, does then the screen read the missing keyboard? or?

Mouse over doesn’t work for keyboard or mobile users. Also it’s very easy to hover away by accident and then you lose the menu. Best to just do it on click. Then it’s easy to add the necessary ARIA attributes as well.

There’s been a fair bit of research on this (see links in answer to this question for just some of them). Bootstrap 4 for example moved from hover menus to click menus.

@shantsis
Copy link
Contributor

Can’t a mouse over action also be triggered by on keyboard focus? It may be a bit more work but doesn’t have to be a click.

@tunetheweb
Copy link
Member

Yes it can but doesn’t address mobile or the ARIA requirements. The reality is we need JavaScript to make accessible menus at present. And, given that this is a progressive enhancement of the “Explore the results” section that’s not a bad thing. So what’s the reason for not using JavaScript?

@tunetheweb
Copy link
Member

Interesting article on this topic which has just been published: https://www.smashingmagazine.com/2021/06/css-javascript-requirements-accessible-components/

@tunetheweb
Copy link
Member

tunetheweb commented Jun 14, 2021

This pull request is dragging out, and growing arms and legs as we add more features, complexity and opinions to it. I'm thinking we should merge it as is, and then iterate upon it. What do you think @AbbyTsai / @rviscomi / @shantsis ?

As a reminder the current code in this branch is basically a floating comments widget for mobile and desktop and some extra calls to action in the Explore the Results section. It can be seen here: https://20210614t202814-dot-webalmanac.uk.r.appspot.com/en/2020/fonts.

I think these are good features, that are beneficial and are mergeable as is. We can add the extra ideas (expandable menu with more options) in a new branch and pull request.

One thing I'd like to do is before merging, is internationalise it, but I am happy to do that and commit to this branch if you've not moved it forward offline @AbbyTsai ?

Let me know your thoughts.

@shantsis
Copy link
Contributor

Design wise it seems fine for now. I was trying it on my mobile and was struggling quite a lot to activate it - see the video linked. https://drive.google.com/file/d/1Fhh9gR7zi50X3KdlolNYE6KEflFzyShf/view?usp=drivesdk

@tunetheweb
Copy link
Member

Oh good spot! Moving to bottom we can make it work regardless of whether menu is showing or not.

Try this: https://20210614t205125-dot-webalmanac.uk.r.appspot.com/en/2020/fonts

@shantsis
Copy link
Contributor

Much better thanks 😊

@rviscomi
Copy link
Member

Merge and iterate SGTM!

@tunetheweb
Copy link
Member

I've added the internalisation pieces so think this is good to merge now.

@AbbyTsai let me know your thoughts before I do that.

@tunetheweb tunetheweb marked this pull request as ready for review June 14, 2021 23:32
@tunetheweb
Copy link
Member

Oh and latest staged version: https://20210615t003135-dot-webalmanac.uk.r.appspot.com/en/2020/fonts

@AbbyTsai
Copy link
Contributor Author

Great to see it and warm thanks to all support by Barry, Shantsis, Rick.

2021 automation moved this from TODO to In Progress Jun 15, 2021
@tunetheweb tunetheweb merged commit 63192c4 into main Jun 15, 2021
2021 automation moved this from In Progress to Done Jun 15, 2021
@tunetheweb tunetheweb deleted the chapter-CTAs-position branch June 15, 2021 16:14
@tunetheweb
Copy link
Member

And it's live! Thanks @AbbyTsai (and @shantsis / @rviscomi for input).

Will be interesting to see if comments increase.
And also Edits/Translations.
Though it is the quiet time between editions so readership is traditionally down at these times.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design Creating the Almanac UX development Building the Almanac tech stack
Projects
2021
Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants