XWIKI 21833 - Provide better controls for editing the profile picture (avatar) #2862

tkrieck · 2024-02-07T13:52:05Z

Jira URL

https://jira.xwiki.org/browse/XWIKI-21833

Changes

Description

Changes the edit photo icon on the profile picture to a link beneath said photo.
This was done because:
- The current icon is a hardcoded reference to a silk icon.
- The current icon is placed in a way that's difficult to see for new users, as it does not follow the standard icon from the IconTheme.
Updated the design of the profile picture to keep it aligned with the profile menu
- Removed box shadow and paddings

Clarifications

I started this work because it is small enough so it could be a nice introduction to contributing code to XWiki.
As stated in the description we have hardcoded references to silk icon, I could try to update it to use the IconTheme, however this would need to be changed on the component AttachmentSelector so it's a deeper change.

Screenshots & Video

Before

After

Executed Tests

Expected merging strategy

Prefers squash: Yes
Backport on branches:
*

…(avatar) * Removed hardcoded reference to a Silk icon * Restyled the button to look like a link * Restyled the CSS on the profile image to clean it up

…(avatar) * Updated font-size

Sereza7 · 2024-02-09T15:47:26Z

...form-user-profile/xwiki-platform-user-profile-ui/src/main/resources/XWiki/XWikiUserSheet.xml

+ .buttonwrapper {
+ a.button {
+ background-color: inherit;
+ background-image: none;
+ border-color: unset;
+ border-radius: unset;
+ border: unset;
+ color: @link-color;
+ cursor: pointer;
+ display: unset;
+ font-size: @font-size-small;
+ font-weight: normal;
+ line-height: unset;
+ margin: unset;
+ padding: unset;
+ text-align: center;
+ vertical-align: middle;


This is some LESS code, so you can probably use mixins https://lesscss.org/features/#mixins-feature to simplify this.
.btn; .btn-default;
will probably do the trick :)
See for example

xwiki-platform/xwiki-platform-core/xwiki-platform-flamingo/xwiki-platform-flamingo-skin/xwiki-platform-flamingo-skin-resources/src/main/resources/flamingo/less/buttons.less

Lines 104 to 112 in 0fbc4cc

input.button, .buttonwrapper button, .buttonwrapper a {

background-image: none; // XWIKI-7870

.btn;

.btn-primary;

}

.buttonwrapper input.secondary, .buttonwrapper button.secondary, .buttonwrapper a.secondary {

.btn-default;

}

Forgot to explain it in the first comment, but IMO this is a button (action links very close to buttons semantically), so we should use a button presentation. .btn-default is pretty easy on the eye with the Iceberg color theme.

A lot of 'buttons' in the XWiki UI are not HTML buttons but actually action links on which we put these styles from bootstrap.

So, I tried using this mixin however IMO this causes the button to grab too much attention, that's why I was unsetting a lot of styles back to their defaults. As far as I know, we don't have a "tertiary" button style, have we?

No we don't have a tertiary button style.
Even the secondary button style is just the default color scheme ^^'
With this mixing, a nice thing is that you will probably need to unset less properties, and be consistent for all the ones you don't want to reset.
E.g. font-size, line-height, font-weight probably do not need to be unset anymore. :)

Nice, I will update these styles with the mixin and probably adjust it a little bit. Thanks!

I updated the styles a little bit using mixins and indeed the CSS is better managed this way. However, in doing so the styles didn't quite fit the photo above it, they seemed a little "sparse" and unintegrated. So I did a few more changes in the styles to keep them contained in a single element. IMO, it looks better while still maintaining the overall look of the theme. What do you think?

I also took some screenshots with different color themes to check them out.

Default Iceberg:

Other themes:

One thing I noticed (but unrelated to this Jira) is that we have quite a few of button styles hurting our consistency, I will open a new Jira for this.

So I did a few more changes in the styles to keep them contained in a single element.

Looking great!
Just as a side note, in order to keep two blocks together visually, you can "merge" their boxes by removing top/bottom padding and the border-radius in the right corners. See the styles of the Settings menu just under the picture for example.

One thing I noticed (but unrelated to this Jira) is that we have quite a few of button styles hurting our consistency, I will open a new Jira for this.

Looking forward to more detail about this 👍

Just as a side note, in order to keep two blocks together visually, you can "merge" their boxes by removing top/bottom padding and the border-radius in the right corners.

Nice, the results turned out to be better with your suggestion, thanks! I will push them after some more tests with different themes and pictures.

...form-user-profile/xwiki-platform-user-profile-ui/src/main/resources/XWiki/XWikiUserSheet.xml

Sereza7 · 2024-02-09T16:10:19Z

Good job!
I pointed out a couple places where you could improve codestyle, but the result looks good 👍

About the PR template, I personally use ##Description for the facts and only put everything related to the thinking process in ##Clarifications.

For your Jira issue, from what I remember it's nice to put an Affected version as soon as you open it (I checked it out and was surprised to see that we don't have a jira best practice about that). For improvements, I usually just put the last release as the Affected version. Here it'd be 16.0.0 :)

Side note: I don't think this PR collides with #2771 , even though it's pretty close.

tkrieck · 2024-02-12T11:23:31Z

Good job! I pointed out a couple places where you could improve codestyle, but the result looks good 👍

About the PR template, I personally use ##Description for the facts and only put everything related to the thinking process in ##Clarifications.

For your Jira issue, from what I remember it's nice to put an Affected version as soon as you open it (I checked it out and was surprised to see that we don't have a jira best practice about that). For improvements, I usually just put the last release as the Affected version. Here it'd be 16.0.0 :)

Side note: I don't think this PR collides with #2771 , even though it's pretty close.

Thank you @Sereza7 I will update the PR and Jira.

…(avatar) * Updated code style

…(avatar) * Updated #avatar styles to keep it visually integrated with the button; * Updated button styles using mixins and a few layout rules to keep alignment;

Sereza7 · 2024-02-14T12:37:08Z

...form-user-profile/xwiki-platform-user-profile-ui/src/main/resources/XWiki/XWikiUserSheet.xml

+ width: ~"calc(100% - 0.6em)";
+ margin: 0 0.3em 0.3em 0.3em;


I was afraid that this calc would break responsiveness but apparently it's okay.

Instead of using calc and hard coded values, you can also use display: flex on the parent (.wikilink here) and flex-grow on the a.button . The button will grow to fill the whole line.

See https://css-tricks.com/snippets/css/a-guide-to-flexbox/ , flex is a really powerful tool :)

Updated with 2b2162f

...form-user-profile/xwiki-platform-user-profile-ui/src/main/resources/XWiki/XWikiUserSheet.xml

…(avatar) * Updated avatar and buttons styles * Updated the button container with flexbox layout instead of using calc()

Sereza7 · 2024-02-14T16:37:00Z

...form-user-profile/xwiki-platform-user-profile-ui/src/main/resources/XWiki/XWikiUserSheet.xml

+ p {
+ text-align: center;
+ }
+ img{


We should add a space here to keep consistency in the codestyle:

put a space between selector and declaration start, ex. "a {}"

from https://dev.xwiki.org/xwiki/bin/view/Community/CodeStyle/XhtmlCssCodeStyle/

Sereza7 · 2024-02-14T16:37:14Z

...form-user-profile/xwiki-platform-user-profile-ui/src/main/resources/XWiki/XWikiUserSheet.xml

- z-index: 1;
+.attachment-picker {
+ .buttonwrapper {
+ .wikilink{


We should add a space here to keep consistency in the codestyle:

put a space between selector and declaration start, ex. "a {}"

from https://dev.xwiki.org/xwiki/bin/view/Community/CodeStyle/XhtmlCssCodeStyle/

So, here each rule also need to have a newline between them?

Also, seeing the rest of this file, a lot of rules are not being followed, should I update the rest of the file as well?

So, here each rule also need to have a newline between them?

Those are not next to each other but nested so it's not mandatory. As far as I know we usually don't do it for nested rulesets.

Also, seeing the rest of this file, a lot of rules are not being followed, should I update the rest of the file as well?

From what I remember, it's okay to update codestyle in a file you're already editing.
If you want to do it, go for it, but there's no requirement as long as it's not code you've updated :)

I adjusted my code with df059ae . I will do it for the rest of the file, but on a separate commit.

Sereza7 · 2024-02-14T16:39:43Z

...form-user-profile/xwiki-platform-user-profile-ui/src/main/resources/XWiki/XWikiUserSheet.xml

+#avatar {
+ p {
+ text-align: center;
+ }


Rulesets that are next to each other should be separated with an empty line. Here, this would mean adding back a line between the p ruleset and the img one :)

Separate adjacent rulesets with an empty line.

From https://dev.xwiki.org/xwiki/bin/view/Community/CodeStyle/XhtmlCssCodeStyle/

PS: It's a pretty new rule in the codestyle, so if you notice it's not followed somewhere, you're welcome to propose a fix for the file :)

Adjusted with df059ae

…(avatar) * Codestyle update

michitux · 2024-07-29T10:51:57Z

...form-user-profile/xwiki-platform-user-profile-ui/src/main/resources/XWiki/XWikiUserSheet.xml

- width: 18px !important;
- z-index: 1;
+.attachment-picker {
+ .buttonwrapper {


There is an unchanged style rule for .attachment-picker .buttonwrapper right before this one, is it intentional that they are kept separate? Also, I'm wondering a bit if these style rules aren't too general (not really changed by this PR), and won't affect other attachment pickers that could, e.g., be present in the user profile's content.

There is an unchanged style rule for .attachment-picker .buttonwrapper right before this one, is it intentional that they are kept separate? Also, I'm wondering a bit if these style rules aren't too general (not really changed by this PR), and won't affect other attachment pickers that could, e.g., be present in the user profile's content.

It's not intentional, from what I can see there's no reason to not include it in my changed code and remove from the first one. I'll do it.

Regarding the generality of rules, maybe we can add a general prefix to all rules and make them more specific. I will do some tests on this.

Thanks for the feedback.

tkrieck added 2 commits February 7, 2024 10:32

XWIKI-21833: Provide better controls for editing the profile picture …

10c083b

…(avatar) * Removed hardcoded reference to a Silk icon * Restyled the button to look like a link * Restyled the CSS on the profile image to clean it up

XWIKI-21833: Provide better controls for editing the profile picture …

ed91fe4

…(avatar) * Updated font-size

Sereza7 reviewed Feb 9, 2024

View reviewed changes

...form-user-profile/xwiki-platform-user-profile-ui/src/main/resources/XWiki/XWikiUserSheet.xml Outdated Show resolved Hide resolved

tkrieck added 2 commits February 13, 2024 14:05

XWIKI-21833: Provide better controls for editing the profile picture …

6a9947e

…(avatar) * Updated code style

XWIKI-21833: Provide better controls for editing the profile picture …

1388fe5

…(avatar) * Updated #avatar styles to keep it visually integrated with the button; * Updated button styles using mixins and a few layout rules to keep alignment;

Sereza7 reviewed Feb 14, 2024

View reviewed changes

...form-user-profile/xwiki-platform-user-profile-ui/src/main/resources/XWiki/XWikiUserSheet.xml Show resolved Hide resolved

XWIKI-21833: Provide better controls for editing the profile picture …

2b2162f

…(avatar) * Updated avatar and buttons styles * Updated the button container with flexbox layout instead of using calc()

Sereza7 reviewed Feb 14, 2024

View reviewed changes

tkrieck added 2 commits February 14, 2024 14:02

XWIKI-21833: Provide better controls for editing the profile picture …

df059ae

…(avatar) * Codestyle update

XWIKI-21833: Provide better controls for editing the profile picture …

7029496

…(avatar) * Codestyle update

tkrieck marked this pull request as ready for review February 21, 2024 09:19

tkrieck and others added 2 commits July 16, 2024 12:41

Merge branch 'master' into XWIKI-21833

62f55fe

Merge branch 'xwiki:master' into XWIKI-21833

bcfb5ba

michitux reviewed Jul 29, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

XWIKI 21833 - Provide better controls for editing the profile picture (avatar) #2862

XWIKI 21833 - Provide better controls for editing the profile picture (avatar) #2862

tkrieck commented Feb 7, 2024

Sereza7 Feb 9, 2024 •

edited

Loading

Sereza7 Feb 9, 2024

tkrieck Feb 13, 2024

Sereza7 Feb 13, 2024

tkrieck Feb 13, 2024

tkrieck Feb 14, 2024

Sereza7 Feb 14, 2024

tkrieck Feb 14, 2024

Sereza7 commented Feb 9, 2024

tkrieck commented Feb 12, 2024

Sereza7 Feb 14, 2024

tkrieck Feb 14, 2024

Sereza7 Feb 14, 2024

Sereza7 Feb 14, 2024

tkrieck Feb 14, 2024

tkrieck Feb 14, 2024

Sereza7 Feb 14, 2024

Sereza7 Feb 14, 2024

tkrieck Feb 14, 2024

Sereza7 Feb 14, 2024 •

edited

Loading

tkrieck Feb 14, 2024

michitux Jul 29, 2024

tkrieck Aug 2, 2024

	input.button, .buttonwrapper button, .buttonwrapper a {
	background-image: none; // XWIKI-7870
	.btn;
	.btn-primary;
	}

	.buttonwrapper input.secondary, .buttonwrapper button.secondary, .buttonwrapper a.secondary {
	.btn-default;
	}

XWIKI 21833 - Provide better controls for editing the profile picture (avatar) #2862

Are you sure you want to change the base?

XWIKI 21833 - Provide better controls for editing the profile picture (avatar) #2862

Conversation

tkrieck commented Feb 7, 2024

Jira URL

Changes

Description

Clarifications

Screenshots & Video

Executed Tests

Expected merging strategy

Sereza7 Feb 9, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Sereza7 commented Feb 9, 2024

tkrieck commented Feb 12, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Sereza7 Feb 14, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Sereza7 Feb 9, 2024 •

edited

Loading

Sereza7 Feb 14, 2024 •

edited

Loading