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

Implement loading icon or hover effect #82

Merged
merged 9 commits into from
May 16, 2023

Conversation

bcgv123
Copy link
Contributor

@bcgv123 bcgv123 commented May 11, 2023

Description

https://apps.nrs.gov.bc.ca/int/jira/browse/SHOWCASE-3167

Hover effect: css has been used as the last tool, as font-awesome-icons do not support hover animation.
Loading icon: primevue spinner has been used for Bucket and Object tables.

Fix after Tyler's comment:
Screenshot 2023-05-12 at 3 06 08 PM

Types of changes

Checklist

  • I have read the CONTRIBUTING doc
  • I have checked that unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

Further comments

@bcgv123 bcgv123 force-pushed the feat/3167-hover-effect-loading-icon branch from 52c44a3 to 1142b7f Compare May 11, 2023 18:12
@github-actions
Copy link

Coverage Report (Application)

Totals Coverage
Statements: 75% ( 51 / 68 )
Methods: 62.5% ( 5 / 8 )
Lines: 82.61% ( 38 / 46 )
Branches: 57.14% ( 8 / 14 )

@github-actions
Copy link

github-actions bot commented May 11, 2023

Coverage Report (Frontend)

Totals Coverage
Statements: 41.17% ( 401 / 974 )
Methods: 39.91% ( 91 / 228 )
Lines: 49.65% ( 287 / 578 )
Branches: 13.69% ( 23 / 168 )

@bcgv123 bcgv123 requested a review from tpantella May 11, 2023 18:23
Copy link

@tpantella tpantella left a comment

Choose a reason for hiding this comment

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

Reviewed using https://bcbox-dev-pr-82.apps.silver.devops.gov.bc.ca/ as provided by Dragan.

Hover appears good, a spot test around various actions seem appropriately subtle as a slight difference in lightness/darkness, agree with the dev choice to lighten darker elements on hover.

Loading animation of table appears to be red circle. Red usually indicates failure and is not a prominent colour in our scheme, not sure if this is an error or intentional but loading should be a shade of blue or grey for consistency. Table also briefly loads with large dark grey dectangle, would prefer a subtle loading colour more congruent with the design such as a white.

Attaching image I was able to capture of red spinner, it's tough to get because it's so quick so you only see part of it.

PR Hover Loading Review TP

@bcgv123
Copy link
Contributor Author

bcgv123 commented May 12, 2023

Reviewed using https://bcbox-dev-pr-82.apps.silver.devops.gov.bc.ca/ as provided by Dragan.

Hover appears good, a spot test around various actions seem appropriately subtle as a slight difference in lightness/darkness, agree with the dev choice to lighten darker elements on hover.

Loading animation of table appears to be red circle. Red usually indicates failure and is not a prominent colour in our scheme, not sure if this is an error or intentional but loading should be a shade of blue or grey for consistency. Table also briefly loads with large dark grey dectangle, would prefer a subtle loading colour more congruent with the design such as a white.

Attaching image I was able to capture of red spinner, it's tough to get because it's so quick so you only see part of it.

PR Hover Loading Review TP

As requested, overwriting spinner colour to grey and loading overlay to white.

@bcgv123 bcgv123 requested review from tpantella and TimCsaky May 12, 2023 22:11
frontend/src/components/layout/Loader.vue Outdated Show resolved Hide resolved
frontend/src/assets/main.scss Outdated Show resolved Hide resolved
@TimCsaky TimCsaky force-pushed the feat/3167-hover-effect-loading-icon branch from 81ace0e to 838bda3 Compare May 16, 2023 22:12
@TimCsaky TimCsaky force-pushed the feat/3167-hover-effect-loading-icon branch from 838bda3 to aa5d921 Compare May 16, 2023 22:33
@jujaga jujaga merged commit da3fa47 into master May 16, 2023
@jujaga jujaga deleted the feat/3167-hover-effect-loading-icon branch May 16, 2023 23:13
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.

5 participants