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
Merged

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

Changes from 5 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
52 changes: 30 additions & 22 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 @@ -448,17 +448,23 @@ return XWiki;
width: 13em;
}

#avatar p {
text-align: center;
}
#avatar {
p {
text-align: center;
}
Copy link
Contributor

@Sereza7 Sereza7 Feb 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adjusted with df059ae

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
border-top: @border-width solid @list-group-border;
border-left: @border-width solid @list-group-border;
border-right: @border-width solid @list-group-border;
border-bottom: 0;
margin: 0 auto;
}

#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-radius: @border-radius-base;
padding: @border-width;
}
}

.profile-menu .category-tab:before{
Expand Down Expand Up @@ -661,18 +667,20 @@ 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.

.wikilink{
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/

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, here each rule also need to have a newline between 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.

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?

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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

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 adjusted my code with df059ae . I will do it for the rest of the file, but on a separate commit.

display: flex;
flex-flow: column;
}
a.button {
.btn;
.btn-default;
flex-grow: 1;
margin: 0;
border-radius: 0 0 @border-radius-base @border-radius-base;
}
}
}

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