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

Jump to bottom

CSS changes for better distinction between user name and full name in user list #33669

Closed
wants to merge 11 commits into from
Closed

CSS changes for better distinction between user name and full name in user list #33669

wants to merge 11 commits into from

Conversation

Jerome-Herbinet
Copy link
Member

2022-08-24_10-39

Signed-off-by: Jérôme Herbinet jerome.herbinet@arawa.fr

Signed-off-by: Jérôme Herbinet 33763786+Jerome-Herbinet@users.noreply.github.com

@Jerome-Herbinet
CSS changes for better visual distinction between user name and full …
8316f49 
…name

Signed-off-by: Jérôme Herbinet jerome.herbinet@arawa.fr

Signed-off-by: Jérôme Herbinet <33763786+Jerome-Herbinet@users.noreply.github.com>
@Jerome-Herbinet Jerome-Herbinet changed the title CSS changes for better distinction between user name and full name CSS changes for better distinction between user name and full name in user list Aug 24, 2022
@szaimen szaimen added enhancement design Design, UI, UX, etc. 3. to review Waiting for reviews labels Aug 24, 2022
@szaimen szaimen added this to the Nextcloud 25 milestone Aug 24, 2022
@szaimen szaimen requested review from juliushaertl, a team, PVince81, artonge and skjnldsv and removed request for a team August 24, 2022 10:01
szaimen
szaimen approved these changes Aug 24, 2022
View reviewed changes
Copy link
Contributor

@szaimen szaimen left a comment

Choose a reason for hiding this comment

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

Lgtm but didn't test

@Jerome-Herbinet
Revert "Fix : Tabs deletion"
f75f575 
This reverts commit 6879c01.
@Jerome-Herbinet
Fix : Deleting some tabs (code cleaning)
109f0b8 
Signed-off-by: Jérôme Herbinet jerome.herbinet@arawa.fr
@Jerome-Herbinet
Copy link
Member Author

What is in the commit branch ? is this normal ? any problem ?
3rdparty
Viewed
Submodule 3rdparty updated 164 files

jancborchardt
jancborchardt requested changes Aug 24, 2022
View reviewed changes
Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

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

Good idea! Some design feedback:

  • We never use italic cause it is not so easily legible
  • Instead, we could bold the display name
  • Display name should show in first row, then account name, so it doesn't look weird with a bold subline
  • That variable looks non-standard, we only use either color-main-text or color-text-maxcontrast. But if we bold the display name, we don't need to reduce the color of the account name.

@szaimen
Copy link
Contributor

szaimen commented Aug 24, 2022

What is in the commit branch ? is this normal ? any problem ? 3rdparty Viewed Submodule 3rdparty updated 164 files

This change needs to be reverted. See https://stackoverflow.com/questions/30275691/removing-subproject-commit-from-github/30275888#30275888

@blizzz blizzz mentioned this pull request Aug 24, 2022
Merged
@Jerome-Herbinet
Copy link
Member Author

Jerome-Herbinet commented Aug 25, 2022 •
edited
Loading

@szaimen I think I've removed it ; can you tell me if it's OK ? (submodule)

@Jerome-Herbinet
Copy link
Member Author

Good idea! Some design feedback:

* [ ]  We never use italic cause it is not so easily legible

* [ ]  Instead, we could bold the display name

* [ ]  Display name should show in first row, then account name, so it doesn't look weird with a bold subline

* [ ]  That variable looks non-standard, we only use either color-main-text or color-text-maxcontrast. But if we bold the display name, we don't need to reduce the color of the account name.

@jancborchardt thanks for your suggestions ; I'll take care of it.

@Jerome-Herbinet
Revert "Removing submodule"
3f0ca0d 
This reverts commit 3e2190e.
@Jerome-Herbinet
Revert "Revert "Removing submodule""
f30d4ae 
This reverts commit 3f0ca0d.
@Jerome-Herbinet
Folling jancborchardt requests with some comments
7fca400 
@jancborchardt
Italic removed. Bold "only" is not enough.
Bold AND "color-main-text" is too much (too aggressive).
So I propose only using color-main-text.
I think I don't have skills to invert lines (and I'm afraid of doing this myself now) ; can someone else do it ?

Signed-off-by: Jérôme Herbinet jerome.herbinet@arawa.fr
@Jerome-Herbinet
Copy link
Member Author

  • following

szaimen
szaimen reviewed Aug 25, 2022
View reviewed changes
.gitmodules
Comment on lines -1 to -3
[submodule "3rdparty"]
path = 3rdparty
url = https://github.com/nextcloud/3rdparty.git
Copy link
Contributor

Choose a reason for hiding this comment

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

hm... seems like following my link removed too much :/

@skjnldsv do you know how to fix that?

Copy link
Member

Choose a reason for hiding this comment

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

Nope :(

Copy link
Member Author

Choose a reason for hiding this comment

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

@StCyr une idée ?

@szaimen
Copy link
Contributor

szaimen commented Aug 25, 2022

@Jerome-Herbinet thanks! Can you please post a new screenshot? That would be great!

@Jerome-Herbinet
Copy link
Member Author

@Jerome-Herbinet thanks! Can you please post a new screenshot? That would be great!

@szaimen here it is :
2022-08-25_15-29

@szaimen
Copy link
Contributor

szaimen commented Aug 25, 2022

cc @jancborchardt

@jancborchardt
Copy link
Member

@Jerome-Herbinet almost :) could you:

  • Unbold the account name, as the display name is already bold
  • Put display name in the first line, accoint in the second (not soo important if difficult)

@Jerome-Herbinet
Copy link
Member Author

@Jerome-Herbinet almost :) could you:

* Unbold the account name, as the display name is already bold

* Put display name in the first line, accoint in the second (not soo important if difficult)

@jancborchardt in fact the text isn't bold ; the other screenshot boldness is certainly due to the default font of my Ubuntu Laptop.

Here is a screenshot of the same NC instance, under Manjaro.

scr

So do you want me to bold something ?

And about putting the display name in first position, I won't be able to do it my self (sorry).

@jancborchardt
Copy link
Member

@Jerome-Herbinet ah got it, thanks for the screenshot! Then I'd only say it would be nice to bold the display name (best use a <strong> tag for semantic markup).

@Jerome-Herbinet
Wider css class call
bc66d1f 
Remove "displayName" class call to target "subtitle" class in users list AND in table header.
Signed-off-by: Jérôme Herbinet jerome.herbinet@arawa.fr
Jerome-Herbinet added a commit to Jerome-Herbinet/server that referenced this pull request Aug 26, 2022
@Jerome-Herbinet
Invert display and user name and bold display name
5f9962d 
For PR nextcloud#33669
Signed-off-by: Jérôme Herbinet jerome.herbinet@arawa.fr

Signed-off-by: Jérôme Herbinet <33763786+Jerome-Herbinet@users.noreply.github.com>
@Jerome-Herbinet
Invert user and display name (+ bold display name)
df5937f 
Signed-off-by: Jérôme Herbinet jerome.herbinet@arawa.fr
@Jerome-Herbinet
Header : Invert display (+ bold) and user name
0aa0d89 
Signed-off-by: Jérôme Herbinet jerome.herbinet@arawa.fr
@Jerome-Herbinet
Copy link
Member Author

Jerome-Herbinet commented Aug 26, 2022 •
edited
Loading

@Jerome-Herbinet ah got it, thanks for the screenshot! Then I'd only say it would be nice to bold the display name (best use a \<strong\> tag for semantic markup).

Done @jancborchardt
2022-08-26_10-12
Display name is bold ... event if on my "day" computer, difference between bold and not bold texte isn't very visible ... (with "strong") and display name and user name are inverted now.

@Jerome-Herbinet
Copy link
Member Author

As a newbie as code contributor, I made an error, by committing 5f9962d ... I think I've cancelled it, can you confirm ? (sorry for the mess, I'm learning).

@Jerome-Herbinet
Copy link
Member Author

I'm concerned about submodule problem ; do you have a tip to solve it ?

This was referenced Aug 30, 2022
Merged
Merged
@blizzz blizzz mentioned this pull request Sep 9, 2022
Merged
@skjnldsv skjnldsv mentioned this pull request Sep 15, 2022
Merged
@Jerome-Herbinet
Copy link
Member Author

Hello,
If the Submodule problem is not solvable, do you advise me to close this PR and to create a new one (hopping that it will work better) ?

@szaimen
Copy link
Contributor

szaimen commented Sep 20, 2022

If the Submodule problem is not solvable, do you advise me to close this PR and to create a new one (hopping that it will work better) ?

Yesy that is probably a good idea. Thanks!

@Jerome-Herbinet
Copy link
Member Author

If the Submodule problem is not solvable, do you advise me to close this PR and to create a new one (hopping that it will work better) ?

Yesy that is probably a good idea. Thanks!

OK @szaimen, I close it and tell you when the new PR is created (I'll cross the fingers).

@Jerome-Herbinet Jerome-Herbinet mentioned this pull request Sep 20, 2022
Merged
@Jerome-Herbinet
Copy link
Member Author

New PR --> #34163

@szaimen szaimen removed this from the Nextcloud 25 milestone Sep 20, 2022
@Jerome-Herbinet Jerome-Herbinet deleted the patch-3 branch August 31, 2023 09:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@skjnldsv skjnldsv skjnldsv approved these changes

@szaimen szaimen szaimen approved these changes

@jancborchardt jancborchardt jancborchardt requested changes

@juliushaertl juliushaertl Awaiting requested review from juliushaertl

@PVince81 PVince81 Awaiting requested review from PVince81

@artonge artonge Awaiting requested review from artonge

@nimishavijay nimishavijay Awaiting requested review from nimishavijay

Assignees
No one assigned
Labels
3. to review Waiting for reviews design Design, UI, UX, etc. enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Jerome-Herbinet @szaimen @jancborchardt @skjnldsv