Skip to content

Several QoL (quality of life) improvements#56

Merged
hiimchrislim merged 11 commits intoutmgdsc:dev/gdsc-open-source-2022from
embeddedt:qol-improvements
Jan 11, 2023
Merged

Several QoL (quality of life) improvements#56
hiimchrislim merged 11 commits intoutmgdsc:dev/gdsc-open-source-2022from
embeddedt:qol-improvements

Conversation

@embeddedt
Copy link

@embeddedt embeddedt commented Oct 7, 2022

Proposed Changes

  • Change hover color to be distinct from selection
    • See commit message.
  • Explicitly state when no option is selected on voting page
    • so students know if their selection has not been processed server-side.
  • Send a browser notification when a new question starts
    • I often switch away from my browser window; this will send a notification that should be visible even if the browser isn't.
  • Fix a typo in the README

Tests

Please describe the tests that you ran to verify your changes.

  • Verified that selection still shows the same color
  • Verified that hovering has significant contrast from not hovering
  • Ran a local poll and verified that a notification is sent when a new question starts
  • Ran a local poll and verified that "Selected Option: None" is shown initially, instead of "Selected Option:"
  • Tested on Edge for Windows 11 and verified that notifications are displayed consistently, even the second time

Copy link
Member

@hiimchrislim hiimchrislim left a comment

Choose a reason for hiding this comment

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

Left a few comments to review.

@embeddedt embeddedt requested review from hiimchrislim and removed request for shubhbapna October 9, 2022 13:53
Students were getting confused and thinking they had selected an
option when they were really just hovering on it. Also, touching
the option results in it remaining "hovered" even after the finger
is long gone.
This is done in addition to changing the title as was done previously.
* Use document.visibilityState to determine when the tab is not focused.
  This avoids a race condition where the tab is unfocused before the
  page has loaded, so the app can't detect it. Also, this lets us
  get rid of an event handler.
* Forcefully close the notification that was previously open, to improve
  reliability of seeing the new notification.
@hiimchrislim
Copy link
Member

Will be looking at this during the weekend. (A bit busy at work atm)

@huonggiangbui
Copy link
Member

@hiimchrislim are you able to check this?

@shubhbapna
Copy link
Collaborator

@embeddedt is it possible to open the PR against gdsc open source branch. I want to include this in the next release scheduled for first of week of winter

@embeddedt embeddedt changed the base branch from main to dev/gdsc-open-source-2022 November 4, 2022 18:34
@embeddedt
Copy link
Author

I switched the base and the commits seem to still be intact; let me know if I still need to create a new PR.

Copy link
Collaborator

@shubhbapna shubhbapna left a comment

Choose a reason for hiding this comment

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

LGTM

@hiimchrislim hiimchrislim merged commit 3054f73 into utmgdsc:dev/gdsc-open-source-2022 Jan 11, 2023
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