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.

Sign up for GitHub

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

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

Merged
merged 11 commits into from
Oct 23, 2024
+48 −36
Merged

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

Changes from 2 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
10c083b
XWIKI-21833: Provide better controls for editing the profile picture …
tkrieck Feb 7, 2024
ed91fe4
XWIKI-21833: Provide better controls for editing the profile picture …
tkrieck Feb 7, 2024
6a9947e
XWIKI-21833: Provide better controls for editing the profile picture …
tkrieck Feb 13, 2024
1388fe5
XWIKI-21833: Provide better controls for editing the profile picture …
tkrieck Feb 14, 2024
2b2162f
XWIKI-21833: Provide better controls for editing the profile picture …
tkrieck Feb 14, 2024
df059ae
XWIKI-21833: Provide better controls for editing the profile picture …
tkrieck Feb 14, 2024
7029496
XWIKI-21833: Provide better controls for editing the profile picture …
tkrieck Feb 15, 2024
62f55fe
Merge branch 'master' into XWIKI-21833
tkrieck Jul 16, 2024
bcfb5ba
Merge branch 'xwiki:master' into XWIKI-21833
tkrieck Jul 22, 2024
c7dfa8e
Merge branch 'xwiki:master' into XWIKI-21833
tkrieck Oct 7, 2024
05c2f19
XWIKI-21833: Provide better controls for editing the profile picture …
tkrieck Oct 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 21 additions & 14 deletions ...m-user-profile/xwiki-platform-user-profile-ui/src/main/resources/XWiki/XWikiUserSheet.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -455,10 +455,8 @@ return XWiki;
#avatar img {
border: 1px solid $theme.borderColor;
border-radius: 8px 8px 8px 8px;
box-shadow: 0 1px 2px rgba(0, 0, 0, 0.15);
margin: 0 auto;
padding: 0.3em;
width: 95%;
}

.profile-menu .category-tab:before{
Expand Down Expand Up @@ -661,18 +659,27 @@ span#avatarUpload {
margin: 0;
}

.attachment-picker-start {
background: url("$xwiki.getSkinFile('icons/silk/picture_edit.png')") no-repeat center center $theme.pageContentBackgroundColor !important;
border: 0 none !important;
border-bottom-left-radius: 8px;
height: 18px;
position: absolute;
right: 0;
text-align: left;
text-indent: -9999px;
top: 1px;
width: 18px !important;
z-index: 1;
.attachment-picker {
.buttonwrapper {
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

@tkrieck Do you still plan to add these changes to this PR, or shall we merge it without them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tkrieck Do you still plan to add these changes to this PR, or shall we merge it without them?

My bad, I had totally forgotten about this. Addressed this in my last commit 10c083b.

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;
Copy link
Contributor

@Sereza7 Sereza7 Feb 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

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;
}

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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?

Screenshot 2024-02-13 at 13 56 50

Copy link
Contributor

Choose a reason for hiding this comment

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

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. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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:
Screenshot 2024-02-14 at 07 56 16

Other themes:
Screenshot 2024-02-14 at 07 56 31
Screenshot 2024-02-14 at 07 56 47
Screenshot 2024-02-14 at 07 57 10
Screenshot 2024-02-14 at 07 57 23
Screenshot 2024-02-14 at 07 59 30

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.

Screenshot 2024-02-14 at 08 06 46

Copy link
Contributor

Choose a reason for hiding this comment

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

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 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Screenshot 2024-02-14 at 09 42 23 Screenshot 2024-02-14 at 09 42 36 Screenshot 2024-02-14 at 09 42 59

&:hover{text-decoration: underline};
}
}
}

## --------------------------------------
Expand Down