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

UI: Improve archive help and tooltips. #3350

Conversation

timpwbaker
Copy link

@timpwbaker timpwbaker commented Jan 20, 2017

There is some confusion around what the archive functionality actually
does, and also some concern about maintaining consistency across
tooltip text.

This commit proposes making use of the i18n gem to provide a
dictionary as it ships with activesupport and will be intuitive to those
from a rails background. It would also support providing translations at
some point in future if that were ever required.

It solves issue:
#3349

This is my first (ever) PR to an open source project, encouraged by
Katrina's appearance on the Bike Shed podcast, but you won't hurt my
feelings if there are things that are not correct.

@timpwbaker timpwbaker changed the title 2Update trackler Add dictionary with archive tooltip as first entry Jan 20, 2017
@Insti
Copy link

Insti commented Jan 20, 2017

Thanks @timpwbaker, and welcome.

It would also

These are scary words for me in a pull request.

I appreciate what you're trying to achieve here and agree that it is a good idea.
But adding internationalization support is a completely separate issue and should be handled separately.

I'd rather see a commit that fixed the archive help text issue in the simplest way possible, and just copy/pasting the required text strings to the required places seems much simpler in this case.

Then the string consistency / I18n issues could be handled in their own PRs.

See also: exercism/discussions#113 for some discussion about where the Exercism front end is headed.

@timpwbaker timpwbaker force-pushed the dictionary_with_first_entry_archive_tooltip branch from 272e653 to ed60303 Compare January 20, 2017 16:45
@Insti
Copy link

Insti commented Jan 20, 2017

Thanks, It is also helpful for UI changes to include screenshots to help casual reviewers see the effect of the changes.

@timpwbaker
Copy link
Author

Thanks! I'm away this weekend but I'll add some images on Monday morning.

@Insti Insti changed the title Add dictionary with archive tooltip as first entry UI: Improve archive help and tooltips. Jan 20, 2017
@tejasbubane
Copy link
Member

tejasbubane commented Jan 21, 2017

Thanks @timpwbaker, this looks great! Just one thing, we also need to mention if the archiving is reversible or not - meaning if I can make the archived exercise back to active. Even I don't know if this can be done 😄

@timpwbaker
Copy link
Author

My feeling is that's a wider UX/UI issue. Explaining whether it's reversible or not seems a little too much information for a tooltip. What does everyone else think?

@timpwbaker
Copy link
Author

Screenshots for reference
screen shot 2017-01-23 at 13 02 54
screen shot 2017-01-23 at 13 02 50
screen shot 2017-01-23 at 13 02 39

@tejasbubane
Copy link
Member

My feeling is that's a wider UX/UI issue. Explaining whether it's reversible or not seems a little too much information for a tooltip

Agreed. I guess tooltip is not a good place for this. And unfortunately I cannot think of any other place for this either. Looking for suggestions from others.

@kytrinyx
Copy link
Member

How about adding some text on the dashboard. It currently looks like this:

screen shot 2017-01-28 at 11 34 30 pm

Perhaps if we explain what archiving means there, then we don't need the tooltips.

@timpwbaker timpwbaker force-pushed the dictionary_with_first_entry_archive_tooltip branch from 09ef6d6 to 736b9a1 Compare February 6, 2017 15:17
@timpwbaker
Copy link
Author

timpwbaker commented Feb 6, 2017

Thanks @kytrinyx

I prefer this approach as while I expect - although I could be wrong - most of Exercism's traffic to be desktop (keyboard and mouse) tooltips have very poor usability on touch screens such as tablets and mobiles.

screen shot 2017-02-06 at 15 19 45

screen shot 2017-02-06 at 15 19 54

There is some confusion around what the archive functionality actually
does, and also some concern about maintaining consistency across
tooltip text.

This commit proposes making use of the i18n gem as it ships with
activesupport and will be intuitive to those from a rails background.
It would also support providing translations at some point in future if
that were ever required.

It solves issue:
exercism#3349

This is my first (ever) PR to an open source project, encouraged by
Katrina's appearance on the Bike Shed podcast, but you won't hurt my
feelings if there are things that are not correct.
This commit removes the internationalization element and simply copies
the required text.
This commit removes the tooltips completely in favour of explaining what
archiving does on the dashboard.

I prefer this approach as while I expect most of Exercism's traffic to
be desktop (keyboard and mouse) tooltips have very poor usability on
touch screens such as tablets and mobiles.

It solves issue:
@kytrinyx kytrinyx force-pushed the dictionary_with_first_entry_archive_tooltip branch from 736b9a1 to 757a29e Compare February 12, 2017 21:23
@kytrinyx kytrinyx merged commit 4f6a491 into exercism:master Feb 12, 2017
@kytrinyx
Copy link
Member

Thanks so much for working on this! ✨

Sorry it took so long to get it merged—things have been a bit hectic lately, and I apologize for letting that affect your first foray into open source contribution.

💜 💙 💛

@timpwbaker
Copy link
Author

No problem!

Thanks for being so encouraging. I shall pick up a few more of the issues when I've got some more time.

TB

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants