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

Proposed UI Update #69

Open
theochawla opened this issue Feb 4, 2022 · 25 comments · May be fixed by #70
Open

Proposed UI Update #69

theochawla opened this issue Feb 4, 2022 · 25 comments · May be fixed by #70

Comments

@theochawla
Copy link

I’ve included a new block icon (the notif bell with a stroke through it). I also think the notifications could be condensed by reducing the line spacing and removing the third line of text. I’ve included a text hierarchy with type size and spacing. They are in px right now, would having them in ems or something similar be more helpful? I also think the drop shadow could be a little softer and more centered under the notification to better show its upper boundary.

Screen.Recording.2022-02-04.at.3.21.51.PM.mov

Screen Shot 2022-02-04 at 3 55 42 PM

@andrii-i
Copy link
Collaborator

andrii-i commented Feb 5, 2022

Thanks for the images, they are very helpful.
Having all font sizes in relative units like ems would be super helpful and would make the fonts look better on screens/windows of non-standard size.

@theochawla
Copy link
Author

theochawla commented Feb 7, 2022

Here are the type sizes in ems!
Screen Shot 2022-02-07 at 3 26 24 PM

@andrii-i andrii-i linked a pull request Feb 16, 2022 that will close this issue
@andrii-i
Copy link
Collaborator

andrii-i commented Feb 16, 2022

Angela, let's schedule a UI implementation review for when possible. Key question: Material UI that is used in Notifications UI uses "rem", not "ems" and that's what I used in the implementation. Does it works or recommended sizes should be adjusted?

Current look:
image

@andrii-i
Copy link
Collaborator

andrii-i commented Feb 16, 2022

Wasn't able to update the design of the notification popup (bottom right corner) because we use a 3rd-party extension for it. Created issue #71 to solve down the road.

@theochawla
Copy link
Author

These look great! The line spacing looks a little looser than in the mockup: maybe we could tighten them up? I wonder if I gave you the wrong em/rem values?

@andrii-i
Copy link
Collaborator

Thank you!
Here "rem" is used while you gave "ems" so the discrepancy is probably due to that.

@theochawla
Copy link
Author

Ok, I will get you updated rem values! Right now, do you know what the root size is (the value the rems are being based on)?

@theochawla
Copy link
Author

If it is not specified, I think the default is 16px

@andrii-i
Copy link
Collaborator

I also haven't implemented small icons in notifications (on the left of the title) because to do it I'll have to change/update notification API providing some mechanism for extensions to send their logo with every message or somehow register/cache it on the server.
It's a good-looking functionality tho so I'll think about how to implement it, would create a new issue.

@theochawla
Copy link
Author

Ok, no worries! I want to create a few different kinds of icons for different notifications, so no rush on implementing that. I will update the issue with the icons when I finish them.

@andrii-i
Copy link
Collaborator

If it is not specified, I think the default is 16px

Yes, as far as I understand it's always a 16px base and then each user/browser modifies this base size based on its needs (https://mui.com/customization/typography/#font-size)

@theochawla
Copy link
Author

That makes sense! Thank you :)

@andrii-i
Copy link
Collaborator

Ok, no worries! I want to create a few different kinds of icons for different notifications, so no rush on implementing that. I will update the issue with the icons when I finish them.

It's a very interesting idea that can make a life for extension devs much easier. The problem here is that API would accommodate all kinds of extensions (and therefore message types) so it might be hard to pre-plan for icons. If we could do it though, it would be nice to have a small default library that extension devs could use.

Were you thinking about like creating icons for broad categories?

@theochawla
Copy link
Author

Ok, no worries! I want to create a few different kinds of icons for different notifications, so no rush on implementing that. I will update the issue with the icons when I finish them.

It's a very interesting idea that can make a life for extension devs much easier. The problem here is that API would accommodate all kinds of extensions (and therefore message types) so it might be hard to pre-plan for icons. If we could do it though, it would be nice to have a small default library that extension devs could use.

Were you thinking about like creating icons for broad categories?

Yes! I was thinking maybe one icon for cell execution, one for new user login, and one for any custom notification. I like the idea of a default library, though. Maybe splitting up potential notifs into categories (system updates, comments, etc.) and having at least one catch-all icon?

@theochawla
Copy link
Author

Also: here are the type sizes in rems. Sorry for the confusion, that was totally my bad.
Screen Shot 2022-02-16 at 3 36 58 PM

@andrii-i
Copy link
Collaborator

Ok, no worries! I want to create a few different kinds of icons for different notifications, so no rush on implementing that. I will update the issue with the icons when I finish them.

It's a very interesting idea that can make a life for extension devs much easier. The problem here is that API would accommodate all kinds of extensions (and therefore message types) so it might be hard to pre-plan for icons. If we could do it though, it would be nice to have a small default library that extension devs could use.
Were you thinking about like creating icons for broad categories?

Yes! I was thinking maybe one icon for cell execution, one for new user login, and one for any custom notification. I like the idea of a default library, though. Maybe splitting up potential notifs into categories (system updates, comments, etc.) and having at least one catch-all icon?

Your idea sounds good and is much more manageable in terms of scope. Let's go with it. In the meantime, we can think if these broad categories are even identifiable.

@theochawla
Copy link
Author

Ok, no worries! I want to create a few different kinds of icons for different notifications, so no rush on implementing that. I will update the issue with the icons when I finish them.

It's a very interesting idea that can make a life for extension devs much easier. The problem here is that API would accommodate all kinds of extensions (and therefore message types) so it might be hard to pre-plan for icons. If we could do it though, it would be nice to have a small default library that extension devs could use.
Were you thinking about like creating icons for broad categories?

Yes! I was thinking maybe one icon for cell execution, one for new user login, and one for any custom notification. I like the idea of a default library, though. Maybe splitting up potential notifs into categories (system updates, comments, etc.) and having at least one catch-all icon?

Your idea sounds good and is much more manageable in terms of scope. Let's go with it. In the meantime, we can think if these broad categories are even identifiable.

Definitely. I'll start with the new user, Jupyter & extension updates, comments, and catch-all. If we think of others, I can add them.

@andrii-i
Copy link
Collaborator

Looks like this with provided rem values:
image

@theochawla
Copy link
Author

It's looking better! Ok. If you are up for making these changes now, I think the timestamp could be the same size as the body type. Can we increase the "more notifications" text size to .575 rems? I also think the headings in the interface could be scaled proportionally could you make their size, maybe 1 rem. The line spacing is still looser than expected. Could you try decreasing them each by .25 rems? If you have other things to finish now, feel free to push these to next week.

@andrii-i
Copy link
Collaborator

It's looking better! Ok. If you are up for making these changes now, I think the timestamp could be the same size as the body type. Can we increase the "more notifications" text size to .575 rems? I also think the headings in the interface could be scaled proportionally could you make their size, maybe 1 rem. The line spacing is still looser than expected. Could you try decreasing them each by .25 rems? If you have other things to finish now, feel free to push these to next week.

Ok, I'll get back to tweaking it next week. If it's not too much to ask, updated hierarchy would really help as I'm afraid to get lost in adjustments.

@theochawla
Copy link
Author

It's looking better! Ok. If you are up for making these changes now, I think the timestamp could be the same size as the body type. Can we increase the "more notifications" text size to .575 rems? I also think the headings in the interface could be scaled proportionally could you make their size, maybe 1 rem. The line spacing is still looser than expected. Could you try decreasing them each by .25 rems? If you have other things to finish now, feel free to push these to next week.

Ok, I'll get back to tweaking it next week. If it's not too much to ask, updated hierarchy would really help as I'm afraid to get lost in adjustments.

Definitely! No problem at all. I'll get that to you today :)

@theochawla
Copy link
Author

It's looking better! Ok. If you are up for making these changes now, I think the timestamp could be the same size as the body type. Can we increase the "more notifications" text size to .575 rems? I also think the headings in the interface could be scaled proportionally could you make their size, maybe 1 rem. The line spacing is still looser than expected. Could you try decreasing them each by .25 rems? If you have other things to finish now, feel free to push these to next week.

Ok, I'll get back to tweaking it next week. If it's not too much to ask, updated hierarchy would really help as I'm afraid to get lost in adjustments.

Definitely! No problem at all. I'll get that to you today :)
External Heading. Size 1 rem. Line spacing 1.25 rem. Bold
Heading. Size .75 rems, Line spacing .75 rem. Bold
Action Type. Size .625 rem, Line Spacing .75 rem. Bold
Notif text. .625 rem, Line Spacing .5 rems. Medium
X more notifications. Size .575 rems, Line Spacing .56 ems. Medium
Note: current sizing in this style guide is 1 rem = 16px
Padding around type is .625 rem

@andrii-i
Copy link
Collaborator

Thank you!

@andrii-i
Copy link
Collaborator

andrii-i commented Apr 16, 2022

Latest updated from @angelachawla:

Screen Shot 2022-04-14 at 3 28 50 PM

@andrii-i
Copy link
Collaborator

Implemented update on font sizes:
Screen Shot 2022-04-19 at 11 13 00 PM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants