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

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

Merged
merged 11 commits into from
Oct 23, 2024
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -448,17 +448,21 @@ return XWiki;
width: 13em;
}

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

img{
Copy link
Contributor

Choose a reason for hiding this comment

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

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/

border-radius: @border-radius-base @border-radius-base 0 0;
tkrieck marked this conversation as resolved.
Show resolved Hide resolved
margin: 0 auto;
padding: 0.3em;
}

#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%;
.attachment-picker {
border: @border-width solid @list-group-border;
border-radius: @border-radius-base;
}
}

.profile-menu .category-tab:before{
Expand Down Expand Up @@ -661,18 +665,15 @@ 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 {
.btn;
.btn-default;
width: ~"calc(100% - 0.6em)";
margin: 0 0.3em 0.3em 0.3em;
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated with 2b2162f

}
}
}

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