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

task/FP-327 - Onboarding welcome messages #191

Merged
merged 19 commits into from
Aug 21, 2020
Merged

task/FP-327 - Onboarding welcome messages #191

merged 19 commits into from
Aug 21, 2020

Conversation

jchuahtacc
Copy link
Contributor

@jchuahtacc jchuahtacc commented Aug 20, 2020

Overview:

Add alert messages to each section
Message text edited by @thbrowntacc

PR Status:

  • Ready.
  • Work in Progress.
  • Hold.

Related Jira tickets:

Summary of Changes:

  • Add alert message describing each section to Workbench component
  • Welcome messages are permanently dismissable
  • Welcome message dismissal state is saved in browser localStorage

Testing Steps:

  1. See and dismiss messages in each section
  2. Reload the browser to make sure they stay gone
  3. Reset your saved message state in Chrome by going to Dev Tools > "Application" Tab > Storage - Local Storage - https://cep.dev in the sidebar -- delete the welcomeMessages key

image

UI Photos:

image
image
image
image
image

Notes:

@codecov
Copy link

codecov bot commented Aug 20, 2020

Codecov Report

Merging #191 into master will increase coverage by 4.74%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #191      +/-   ##
==========================================
+ Coverage   58.33%   63.08%   +4.74%     
==========================================
  Files         251      109     -142     
  Lines        8585     2847    -5738     
  Branches     1269      648     -621     
==========================================
- Hits         5008     1796    -3212     
+ Misses       3358      971    -2387     
+ Partials      219       80     -139     
Flag Coverage Δ
#javascript 63.08% <0.00%> (-0.23%) ⬇️
#unittests ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
client/src/components/Workbench/Workbench.js 0.00% <0.00%> (ø)
server/portal/apps/signals/signals.py
server/portal/libs/agave/serializers.py
server/portal/apps/workbench/views.py
server/portal/libs/agave/models/base.py
...ps/workspace/migrations/0002_auto_20200218_2115.py
server/portal/apps/onboarding/api/webhook.py
.../portal/apps/onboarding/migrations/0001_initial.py
server/portal/apps/users/views.py
server/portal/apps/workbench/api/views.py
... and 133 more

@jchuahtacc jchuahtacc changed the title Alert messages task/FP-327 - Onboarding welcome messages Aug 20, 2020
@wesleyboar
Copy link
Member

Please use the info color/style. It's the gray one.

@nathanfranklin nathanfranklin added this to the Grey-Cardinal Release milestone Aug 20, 2020
@jchuahtacc
Copy link
Contributor Author

Please use the info color/style. It's the gray one.

I am. Is this wrong?

            <Alert
              isOpen={welcomeMessages.dashboard}
              toggle={() => onDismissWelcome('dashboard')}
              color="info"
              className="welcomeMessage"
            >

@rstijerina
Copy link
Member

rstijerina commented Aug 20, 2020

Please use the info color/style. It's the gray one.

I am. Is this wrong?

            <Alert
              isOpen={welcomeMessages.dashboard}
              toggle={() => onDismissWelcome('dashboard')}
              color="info"
              className="welcomeMessage"
            >

@tacc-wbomar Referring to the _common Message?
Or the bootstrap alert-secondary?

Copy link
Member

@nathanfranklin nathanfranklin left a comment

Choose a reason for hiding this comment

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

This is great. 👍

@nathanfranklin
Copy link
Member

Local storage is good but do you think people will get annoyed seeing it again when they use a different computer?

@wesleyboar
Copy link
Member

wesleyboar commented Aug 21, 2020

@rstijerina @jchuahtacc
Whoops. My bad. Yes, I meant "secondary". So, color="secondary".

P.S. For the UI-curious, Reactstrap should not have chosen color as the property name. Alerts/messages/notifications/whatever types can be distinguished without color.

@wesleyboar
Copy link
Member

wesleyboar commented Aug 21, 2020

Local storage is good but do you think people will get annoyed seeing it again when they use a different computer?

I vote "They'll live". This is a temp solution overall.

If feasible, then tying the state to user account is "better", but I don't know how to do (thus critique) that, and I assumed Joon Ye found it quicker to use local storage than step into what might be an active cobweb of user data (again, I dunno).

@jchuahtacc
Copy link
Contributor Author

Local storage is good but do you think people will get annoyed seeing it again when they use a different computer?

I vote "They'll live". This is a temp solution overall.

If feasible, then tying the state to user account is "better", but I don't know how to do (thus critique) that, and I assumed Joon Ye found it quicker to use local storage than step into what might be an active cobweb of user data (again, I dunno).

Yeah this is temporary. Ideally we'd tie this into the user profile in django and fold these sagas/reducers together, but in the amount of time that i had this was the most attainable.

Copy link
Member

@rstijerina rstijerina left a comment

Choose a reason for hiding this comment

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

Rad.

Note: Do we also want a welcome message for "Manage Account" and the unauthed tickets view?

Copy link
Member

@wesleyboar wesleyboar left a comment

Choose a reason for hiding this comment

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

Hoowah!

@wesleyboar
Copy link
Member

wesleyboar commented Aug 21, 2020

Rad.

Note: Do we also want a welcome message for "Manage Account" and the unauthed tickets view?

I've asked Maytal directly. These messages were her want.

https://tacc-team.slack.com/archives/GQW4Q8HLG/p1598031117004400

@wesleyboar
Copy link
Member

Yes, let's add one for (logged out) "Add Ticket" form—if feasible in the time available.

Message Proposal:

This page allows you to submit a help request via an RT Ticket.

Why so short (no info about what happens next)?:

  • The form message afterward _tells the user to expect contact.
  • The user will get an e-mail (if they didn't lie about e-mail address).
  • All website tickets work like this... We are allowed to assume some user knowledge.

@wesleyboar
Copy link
Member

And, if Maytal doesn't answer in time (y'all decide what "in time" means), then forgo or add message for Manage Account.

Message Proposal:

This page allows you to A, B, and C.

I don't give much thought to this, because Design didn't want these to begin with.

@rstijerina
Copy link
Member

rstijerina commented Aug 21, 2020

And, if Maytal doesn't answer in time (y'all decide what "in time" means), then forgo or add message for Manage Account.

Message Proposal:

This page allows you to A, B, and C.

I don't give much thought to this, because Design didn't want these to begin with.

For time's sake, let's follow up with these in a separate task.

https://jira.tacc.utexas.edu/browse/FP-652

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.

4 participants