Added clarifying language to the HIG. Sorry about the images.
Details
Diff Detail
- Repository
- R985 KDE Human Interface Guidelines
- Branch
- arcpatch-D14985_1
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 2594 Build 2612: arc lint + arc unit
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) |
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.
Almost there, but you missed one comment: :)
source/introduction/personas.rst | ||
---|---|---|
6 | This comment still hasn't been addressed yet. |
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?
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.
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. | |
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 |
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.
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. |
I suggested to leave the original images in git so we can reuse them and to create scaled down versions for the personas page.
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>
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 :) |
source/style/elevation.rst | ||
---|---|---|
16 | OK, fair enough! :) |
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.
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...
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.
- Fixed wording in 'One-handed use' and 'Depth, Elevation and Shadows'
- More fixes in 'Depth, Elevation and Shadows'