Add basic fields in footer notification styling #971

Ezio-Sarthak · 2021-03-30T04:49:36Z

What does this PR do?!

Adds general styling fields: task:success, task:error, task:warning.
Adds contrast-color style for footer hint text.
refactor: Add default report helpers for success/error/warning footer notifications.
Replace the instances of set_footer_text() with added helpers wherever reasonable.

I'd highly appreciate any feedback :)

Fixes #782.

Rohitth007 · 2021-03-31T12:27:29Z

@Ezio-Sarthak Really nice addition to ZT.
Why area:neutral though? footer should make sense as there is also a header, for the default color.
Also in terms of readability, I feel someone looking at themes.pyfor the very first time would instantly make sense of what a header is and what a footer is than area:neutral.
Just a suggestion :)

Ezio-Sarthak · 2021-03-31T13:14:50Z

@Rohitth007 thanks for the feedback. Re area: neutral, it's true that it wouldn't be readable for footer styling sense, though my intent was that this could be used as a general style elsewhere in the UI in future, if that makes sense?

Rohitth007 · 2021-03-31T14:39:44Z

@Ezio-Sarthak I'm assuming the footer would have an independent styling as it acts sort of like a notification bar?
Also, I think applying same setting to different components reduces independent customizability.

zee-bit

This is really nice work. The footer looks much clean and dynamic now. 👍

I haven't tested it on all themes yet, but the darker ones are looking good.
I am also inclined to Sai's point above that we should have footer instead of area, but the task sounds like a fine replacement too. I would love to gather some more opinions on this, but the PR, in general, should be very close to being merge-able. I have dropped in some points below, but they are mostly minor and may need discussion. :)

zulipterminal/config/themes.py

zee-bit · 2021-04-07T12:35:16Z

zulipterminal/config/themes.py

+        ('area:neutral',   'black',                   'light gray',
+         None,             DEF['black'],              DEF['light_gray']),


We should change this to task, to be more consistent with other footer styles. We could always have another one for area:neutral if the need be.

Thanks for pointing that out. The original intent for it was that it might be used (in general) for other areas (in future) where there's no highlighting required, e.g., typing status. However, since I couldn't think of more such examples, currently we could just align it with other footer-dedicated attributes. We could simply add such themes later in any case 👍

zulipterminal/ui_tools/boxes.py

neiljp

@Ezio-Sarthak Thanks for taking this up - I agree with others that the visual feedback is much better 👍

I've left some comments inline, but I think we'll likely be able to squash these commits together.

Reading this PR it's more clear that we now call a function in a very similar way in many places! I'd consider a refactor before this to add something like report_success and similar methods to the controller which would have the default timeout and know the style to use and where to report it (ie. footer). That would make it less explicit where/how we're reporting it, but remove the need for all the explicit style variables here and the existing time variable (which we could scale up slightly based on the string length?).

This needs a black rebase in any case.

zulipterminal/ui.py

neiljp · 2021-06-03T19:47:33Z

zulipterminal/config/themes.py

+    'task:success': 'standout',
+    'task:failure': 'standout',
+    'task:warning': 'standout',


I'm unsure if having these the same as the footer default provides the (temporary) strikingly visible update that we'll get with the colored themes.

I think having a color (say green) on a result (say success) draws user's attention more than just changing the text with the default footer background. We could surely gain feedback in stream in any case.

Right, my initial thought would be to temporarily do something like invert from the default style; I'm not sure what that translates to. Discussing on the stream would be good, though this would be a small update later too.

zulipterminal/config/themes.py

This commit: * adds styles for various notification categories (inverted color backgrounds). * Improves the default 'footer' style, avoiding conflict with the added styles. The categories with associated styles and footer used as of now: * success (task:success) - on successful completion of a task * warning (task:warning) - providing heads-up for a certain task * error (task:error) - on an errorenous result * neutral (footer) - default notification style

This commit adds entries in themes.py, labelled 'footer_contrast' that define the color for hint text. This could also be referred to as the color scheme for background of the footer. Prior to this an old theme style name was in use, which could give unreliable rendering results between systems. This commit migrates from this old absent "code" style to use the newly added style. Tests amended.

This commit: * amends set_footer_text() to allow styles passed explicitly, with default style set to 'footer'. * adds general helpers for footer notification that avoid need to specify style and duration of the notification. (report_success, report_error, report_warning). Tests amended.

This commit replaces those instances of `set_footer_text` with the new report_* helpers, that notify user of a success/error/warning response. Tests amended. Fixes zulip#782.

neiljp · 2021-06-16T20:32:05Z

@Ezio-Sarthak Thanks for the polish on this; I adapted some of the commit text and have just merged it 🎉

zulipbot added size: L [Automatic label added by zulipbot] enhancement New feature or request further discussion required Discuss this on #zulip-terminal on chat.zulip.org feedback wanted labels Mar 30, 2021

Ezio-Sarthak force-pushed the footer-notification-styling branch 2 times, most recently from 251cf02 to 4a9431e Compare March 31, 2021 02:24

Ezio-Sarthak force-pushed the footer-notification-styling branch from 4a9431e to 51ba272 Compare April 3, 2021 16:15

zee-bit reviewed Apr 7, 2021

View reviewed changes

Ezio-Sarthak force-pushed the footer-notification-styling branch 2 times, most recently from c3240b8 to 9c48738 Compare April 17, 2021 03:35

Ezio-Sarthak force-pushed the footer-notification-styling branch 2 times, most recently from ad7a80c to 8da6281 Compare April 28, 2021 11:24

neiljp reviewed Jun 3, 2021

View reviewed changes

neiljp added the PR awaiting update PR has been reviewed & is awaiting update or response to reviewer feedback label Jun 3, 2021

zulipbot added size: XL [Automatic label added by zulipbot] and removed size: L [Automatic label added by zulipbot] labels Jun 15, 2021

Ezio-Sarthak force-pushed the footer-notification-styling branch 4 times, most recently from 9a34ecd to a8ed8a9 Compare June 16, 2021 18:02

Ezio-Sarthak added PR needs review PR requires feedback to proceed and removed PR awaiting update PR has been reviewed & is awaiting update or response to reviewer feedback labels Jun 16, 2021

Ezio-Sarthak force-pushed the footer-notification-styling branch from a8ed8a9 to 4dd1121 Compare June 16, 2021 18:43

Ezio-Sarthak added 4 commits June 16, 2021 12:15

boxes/buttons/core/helper/model/ui: Use new semantic report_* methods.

5eecc12

This commit replaces those instances of `set_footer_text` with the new report_* helpers, that notify user of a success/error/warning response. Tests amended. Fixes zulip#782.

neiljp force-pushed the footer-notification-styling branch from 4dd1121 to 5eecc12 Compare June 16, 2021 20:19

neiljp merged commit 9eaa6b0 into zulip:main Jun 16, 2021

neiljp removed the PR needs review PR requires feedback to proceed label Jun 16, 2021

neiljp added this to the Next Release milestone Jun 16, 2021

Ezio-Sarthak deleted the footer-notification-styling branch June 17, 2021 04:54

neiljp added the area: UI General user interface update label Feb 28, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add basic fields in footer notification styling #971

Add basic fields in footer notification styling #971

Ezio-Sarthak commented Mar 30, 2021 •

edited

Loading

Rohitth007 commented Mar 31, 2021

Ezio-Sarthak commented Mar 31, 2021

Rohitth007 commented Mar 31, 2021 •

edited

Loading

zee-bit left a comment

zee-bit Apr 7, 2021

Ezio-Sarthak Apr 8, 2021

neiljp left a comment

neiljp Jun 3, 2021

Ezio-Sarthak Jun 14, 2021

neiljp Jun 14, 2021

neiljp commented Jun 16, 2021

		('area:neutral', 'black', 'light gray',
		None, DEF['black'], DEF['light_gray']),

Add basic fields in footer notification styling #971

Add basic fields in footer notification styling #971

Conversation

Ezio-Sarthak commented Mar 30, 2021 • edited Loading

Rohitth007 commented Mar 31, 2021

Ezio-Sarthak commented Mar 31, 2021

Rohitth007 commented Mar 31, 2021 • edited Loading

zee-bit left a comment

Choose a reason for hiding this comment

zee-bit Apr 7, 2021

Choose a reason for hiding this comment

Ezio-Sarthak Apr 8, 2021

Choose a reason for hiding this comment

neiljp left a comment

Choose a reason for hiding this comment

neiljp Jun 3, 2021

Choose a reason for hiding this comment

Ezio-Sarthak Jun 14, 2021

Choose a reason for hiding this comment

neiljp Jun 14, 2021

Choose a reason for hiding this comment

neiljp commented Jun 16, 2021

Ezio-Sarthak commented Mar 30, 2021 •

edited

Loading

Rohitth007 commented Mar 31, 2021 •

edited

Loading