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

Some UX improvements #2425

Merged
merged 9 commits into from
Sep 13, 2022
Merged

Some UX improvements #2425

merged 9 commits into from
Sep 13, 2022

Conversation

glenn2223
Copy link
Contributor

@glenn2223 glenn2223 commented Sep 2, 2022

Sorry, didn't realise rename would close original PR


Made a few UX improvements:

  • The send later delay can be skipped by clicking the new Send now instead button (localization required)
  • If there is a range selection in the subject line then a context menu pops up on right-click (for those that are allergic to keyboard shortcuts 😂)]
  • Dates/times used are now in a localised format, rather than the American one
  • Message participant tweaks:
    • you can now open the context menu on the name or email, not just the email
    • you don't need to expand the participants to open the context menu
    • name is shown over the email for Email "Name else Email" context menu item
    • added Copy "Email" to the context menu when nothing is selected/highlighted and just Copy when something is
  • for devs: pretty console messages now consider your device theme and use an appropriate colour

Made a few UX improvements:
- The send later delay can be skipped by clicking the new `Send now instead` button (localization required)
- If there is a range selection in the subject line then a context menu pops up on right click (for those that are allergic to keyboard shortcuts 😂)
- Message participant tweaks:
  - you can now open the context menu on the name or email, not just the email
  - you don't need to expand the participants to open the context menu
  - name is shown over the email for `Email "Name else Email"` context menu item
  - added `Copy "Email"` to context menu when nothing is selected/highlighted and just `Copy` when something is
- **for devs:** pretty console messages now consider your device theme and use an appropriate colour
- Fix email context menu (now email and name can be alt-clicked)
- Sorted auto format (sorry didn't realise it was off)
Use a localised date format rather than forcing the Americanised format. This does loose the ordinal in `fullTimeString(Date)`
- Changed recent emails to:
  - display weekday and time (e.g Mon, 10:15)
  - the recent emails to be 5 days, rather than 2
- Removed dead code
- Removed unnecessary import
@glenn2223
Copy link
Contributor Author

Not sure why the check failed, built fine on my PC - no lint issues (that weren't already there)

Added a key to prevent a `Each child in an array or iterator should have a unique "key" prop` error in the console
Copy link
Collaborator

@bengotow bengotow left a comment

Choose a reason for hiding this comment

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

This looks great, thanks for contributing these changes - I'm going to add a key to that one React element and merge this so we can ship it in the next release!

if (names.length > max) {
const extra = names.length - max;
names = names.slice(0, max);
names.push(`and ${extra} more`);
names.push(<span>and ${extra} more</span>);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a super minor nit, but I think this line may need a key="contact-more" prop to silence a React warning because it's rendered out of an array.

@@ -389,27 +389,40 @@ const DateUtils = {
*
* The returned date/time format depends on how long ago the timestamp is.
*/
shortTimeString(datetime) {
shortTimeString(datetime: Date) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This all looks great, thank you for updating it to use the new browser standard APIs!

})
);

block.tasks[0].value.expiration = newExpiry;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This looks good, cool that updating the metadata actually causes it to send immediately. I'll try this a couple times once we merge just to stress test it a bit, but it seems safe to me.

@bengotow bengotow merged commit 450dfbe into Foundry376:master Sep 13, 2022
@glenn2223
Copy link
Contributor Author

Thanks @bengotow, happy to contribute. Glad your happy with this and the other 2 merged PRs

Don't suppose you've got any input on my other 2 open PRs 😁

@glenn2223 glenn2223 deleted the ux-improvements branch September 13, 2022 17:36
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 this pull request may close these issues.

2 participants