Revise HIGs for Clarity
ClosedPublic

Authored by abetts on Aug 22 2018, 3:41 AM.

Details

Summary

Added clarifying language to the HIG. Sorry about the images.

Diff Detail

Repository
R985 KDE Human Interface Guidelines
Branch
arcpatch-D14985
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 2625
Build 2643: arc lint + arc unit
abetts requested review of this revision.Aug 22 2018, 3:41 AM
abetts created this revision.
abetts updated this revision to Diff 40183.Aug 22 2018, 4:37 AM

Personas Page Clarifications

ngraham requested changes to this revision.Aug 22 2018, 4:51 PM
ngraham added a subscriber: ngraham.

Nice improvements. Just a few comments below:

source/introduction/personas.rst
6

"Personas is" -> "Personas are" or "Personas provide" or "The concept of Personas provides"

"specific people with specific characteristics that KDE is looking to achieve" -> maybe something more like "the type of people you consider your software's target user."

39

"stupid" -> "limited"

43

"framework" -> "set of tools"

("framework" has a specific technical meaning in KDE)

This revision now requires changes to proceed.Aug 22 2018, 4:51 PM

Will apply those changes. Not a problem!

Also please change the patch title to something that conforms to https://community.kde.org/Infrastructure/Phabricator#Formatting_your_patch

It needs to be a sentence in the imperative mood (e.g. "Clarify language in Personas section")

You can do this using the web interface, after which you will need to run arc amend on the command line.

abetts updated this revision to Diff 40268.Aug 23 2018, 4:36 AM

Clarify Language for Personas

abetts retitled this revision from Clarification Language to Clarify Language for Personas HIG.Aug 23 2018, 4:36 AM
abetts updated this revision to Diff 40269.Aug 23 2018, 4:39 AM
abetts marked 2 inline comments as done.

Clarify Language for Personas HIG

ngraham requested changes to this revision.Aug 23 2018, 3:14 PM

Almost there, but you missed one comment: :)

source/introduction/personas.rst
6

This comment still hasn't been addressed yet.

This revision now requires changes to proceed.Aug 23 2018, 3:14 PM

Almost there, but you missed one comment: :)

Can you help me a little here? I am not finding the line where you want me to use these replacements. Can you please point out the exact sentence?

Almost there, but you missed one comment: :)

Can you help me a little here? I am not finding the line where you want me to use these replacements. Can you please point out the exact sentence?

Line 4 of personas.rst. Just search for "Personas is"

abetts updated this revision to Diff 40446.Aug 25 2018, 11:56 PM

Revise Grammar in One Handed Use

ngraham requested changes to this revision.Aug 26 2018, 4:28 PM

This is now no longer just about the Personas, so the title is no longer accurate. In the future, please submit a separate patch for different things.

This revision now requires changes to proceed.Aug 26 2018, 4:28 PM
abetts updated this revision to Diff 40485.Aug 26 2018, 10:41 PM

Add Introduction to Shadows and Organize Shadow Specifications

abetts edited the summary of this revision. (Show Details)Aug 26 2018, 11:24 PM
fabianr added inline comments.Aug 27 2018, 6:42 AM
source/introduction/convergence.rst
24

I actually think that bigger screens should not necessarily show more controls, but should depend on the used input control too. E.g. a TV witch you navigate with a remote control should not show more controls in my opinion.
So we should not focus just on the screen size, but on the input method too in the next section.

source/introduction/research.rst
93

On the contribute page we link to "Matrix, IRC or Telegram", IO think we should do the same here. You can just copy the content from the contribute.rst

abetts marked 4 inline comments as done.Sep 2 2018, 5:49 AM
abetts updated this revision to Diff 40842.Sep 2 2018, 5:50 AM

Address Comments by Reviewers

ngraham requested changes to this revision.Sep 3 2018, 10:10 PM

Those images are way too big; 1-3mb for each is not a reasonable use of our users' bandwidth. :)

Since this is no longer just about Personas, can you change the title accordingly?

Once the issues with everything here are ironed out, I think we can land it, but in the future, let's make different branches for different changes so we keep separate changes separate. Hit me up if you need help.

This revision now requires changes to proceed.Sep 3 2018, 10:10 PM

Those images are way too big; 1-3mb for each is not a reasonable use of our users' bandwidth. :)

I mentioned this to Fabian. He told me to leave them as is for some reason.

abetts retitled this revision from Clarify Language for Personas HIG to Revise HIGs for Clarity.Sep 3 2018, 10:54 PM
ngraham added a subscriber: zzag.Sep 3 2018, 11:44 PM

Getting there! A few more comments:

source/layout/onehand.rst
24

Unnecessary space after the comma.

28

I know this is a page about phone use, but the phrase "When using Kirigami" seems unnecessarily universal, and makes it seem like maybe this is a general guideline rather than a phone-specific guideline.

source/style/elevation.rst
4–15

"remained using shadows" -> "continued to use shadows" or "retained shadows"

6

"is an intrinsic part" -> "are intrinsic parts" (subject-verb agreement)

16

@zzag could you provide a new screenshot for this as well as accurate values? I wrote this section for the previous new shadows, not the current new shadows. :) So some of the values and the image are out of date now.

Also, I love the new pictures. Very nice.

Those images are way too big; 1-3mb for each is not a reasonable use of our users' bandwidth. :)

Since this is no longer just about Personas, can you change the title accordingly?

Once the issues with everything here are ironed out, I think we can land it, but in the future, let's make different branches for different changes so we keep separate changes separate. Hit me up if you need help.

Those images are way too big; 1-3mb for each is not a reasonable use of our users' bandwidth. :)

I mentioned this to Fabian. He told me to leave them as is for some reason.

I suggested to leave the original images in git so we can reuse them and to create scaled down versions for the personas page.

zzag added a comment.Sep 4 2018, 12:50 PM

Not sure what values to provide because shadows are composed of two. Anyway, here's params for all shadows:

  • Decoration shadows:
.decoration--shadow__small {
    box-shadow: 0 4px 16px rgba(35, 38, 41, 0.6),
                0 2px 4px rgba(35, 38, 41, 0.14);
}

.decoration--shadow__medium {
    box-shadow: 0 8px 32px rgba(35, 38, 41, 0.7),
                0 3px 14px rgba(35, 38, 41, 0.12);
}

.decoration--shadow__large {
    box-shadow: 0 18px 64px rgba(35, 38, 41, 0.8),
                0 8px 24px rgba(35, 38, 41, 0.1);
}

.decoration--shadow__verylarge {
    box-shadow: 0 26px 96px rgba(35, 38, 41, 0.95),
                0 14px 28px rgba(35, 38, 41, 0.1);
}
  • KStyle shadows(tooltip shadows, combo box popup shadows, etc):
.style--shadow__small {
    box-shadow: 0 6px 12px rgba(35, 38, 41, 0.2),
                0 3px 6px rgba(35, 38, 41, 0.16);
}

.style--shadow__medium {
    box-shadow: 0 8px 16px rgba(35, 38, 41, 0.21),
                0 4px 6px rgba(35, 38, 41, 0.14);
}

.style--shadow__large {
    box-shadow: 0 10px 20px rgba(35, 38, 41, 0.23),
                0 5px 8px rgba(35, 38, 41, 0.12);
}


.style--shadow__large {
    box-shadow: 0 12px 24px rgba(35, 38, 41, 0.26),
                0 7px 10px rgba(35, 38, 41, 0.12);
}

You could use those styles, for example, to show shadows, e.g.

<div class="container__row">
    <div class="card__small kstyle--shadow__small"></div>
    <div class="card__small kstyle--shadow__medium"></div>
    <div class="card__small kstyle--shadow__large"></div>
    <div class="card__small kstyle--shadow__verylarge"></div>
</div>

Thanks @zzag!

@abetts, could you also change the shadows section on the elevation page (since you're already touching it :) ) to reflect this information?

abetts added a comment.Sep 4 2018, 2:46 PM

Thanks @zzag!

@abetts, could you also change the shadows section on the elevation page (since you're already touching it :) ) to reflect this information?

I sure can.

In D14985#320162, @zzag wrote:

Not sure what values to provide because shadows are composed of two. Anyway, here's params for all shadows:

  • Decoration shadows:

    `lang=css .decoration--shadow__small { box-shadow: 0 4px 16px rgba(35, 38, 41, 0.6), 0 2px 4px rgba(35, 38, 41, 0.14); }

    .decoration--shadow__medium { box-shadow: 0 8px 32px rgba(35, 38, 41, 0.7), 0 3px 14px rgba(35, 38, 41, 0.12); }

    .decoration--shadow__large { box-shadow: 0 18px 64px rgba(35, 38, 41, 0.8), 0 8px 24px rgba(35, 38, 41, 0.1); }

    .decoration--shadow__verylarge { box-shadow: 0 26px 96px rgba(35, 38, 41, 0.95), 0 14px 28px rgba(35, 38, 41, 0.1); } `
  • KStyle shadows(tooltip shadows, combo box popup shadows, etc):

    `lang=css .style--shadow__small { box-shadow: 0 6px 12px rgba(35, 38, 41, 0.2), 0 3px 6px rgba(35, 38, 41, 0.16); }

    .style--shadow__medium { box-shadow: 0 8px 16px rgba(35, 38, 41, 0.21), 0 4px 6px rgba(35, 38, 41, 0.14); }

    .style--shadow__large { box-shadow: 0 10px 20px rgba(35, 38, 41, 0.23), 0 5px 8px rgba(35, 38, 41, 0.12); }

.style--shadow__large {

box-shadow: 0 12px 24px rgba(35, 38, 41, 0.26),
            0 7px 10px rgba(35, 38, 41, 0.12);

}

You could use those styles, for example, to show shadows, e.g.

```lang=html
<div class="container__row">
    <div class="card__small kstyle--shadow__small"></div>
    <div class="card__small kstyle--shadow__medium"></div>
    <div class="card__small kstyle--shadow__large"></div>
    <div class="card__small kstyle--shadow__verylarge"></div>
</div>

Are there default values for use in qml too?

source/style/elevation.rst
16

Let's change that in a new patch. Otherwise this patch will never finish :)

ngraham added inline comments.Sep 7 2018, 4:28 AM
source/style/elevation.rst
16

OK, fair enough! :)

abetts marked 7 inline comments as done.Sep 8 2018, 4:51 AM

I wasn't able to do arc diff on this, sorry. I committed changes through git. I wasn't able to add the shadows in the markup file but we can address that later.

fabianr updated this revision to Diff 41195.Sep 8 2018, 11:38 AM
  • added small version of personas images and used these in the page
fabianr accepted this revision.Sep 8 2018, 11:40 AM

After talking to Andy,I added the small images to the git and used these in the personas page. But the original images are still in the git, so we can use them else where.

Did this get committed? I don't see it in the git log. Which is good, because my last few comments still haven't been addressed...

abetts added a comment.Sep 9 2018, 5:30 AM

I thought it submitted what I changed and I did address the comments with the new changes. Not sure really what's going on but I can maybe share the actual files and you guys can commit them? I think the real issue is not the comments but my messed up process to submit the information after it has been changed.

fabianr updated this revision to Diff 41245.Sep 9 2018, 8:54 AM
  • Fixed wording in 'One-handed use' and 'Depth, Elevation and Shadows'
  • More fixes in 'Depth, Elevation and Shadows'
ngraham accepted this revision.Sep 9 2018, 11:58 PM

Let's finally land this. :)

This revision is now accepted and ready to land.Sep 9 2018, 11:58 PM

Let's finally land this. :)

Please! Thanks guys. Growing pains but we are moving forward! :D

Wanna do the honors, @fabianr?

fabianr closed this revision.Sep 10 2018, 10:27 AM

Thanks to all for this great patch!