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

24 Hour Merchant Timers #11

Merged
merged 5 commits into from May 7, 2022
Merged

Conversation

alairon
Copy link
Contributor

@alairon alairon commented May 6, 2022

Pull request checklist

Please check if your PR fulfills the following requirements:

  • Docs have been reviewed and added / updated if needed (for bug fixes / features)
  • Build (npm run build) was run locally and any changes were pushed
  • Lint (npm run lint) has passed locally and any fixes were made for failures

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

What is the current behavior?

  • Users are unable to toggle between 12/24 hour formats from within the Merchant settings modal. This provides an inconsistent user experience for users who don't know the setting is under the Alarms settings modal
  • The Settings header in the Merchants modal is hardcoded in English. Translators may wish to have this text translated in the future (issue not present in the Alarms modal)

Issue URL: N/A

What is the new behavior?

  • Improved consistency between the Alarm and Merchant setting modals
    • Added a 12/24 hour toggle to the Merchant setting modal
    • Changed the heading found in the Merchant setting modal to respect locale settings
    • Moved the toggle for local (current)/server time to the same spot so that it matches the position found in the Alarm modal
    • Moved the view-in-24-hr string from public/locales/[en,zh]/alarmConfig.json to public/locales/[en,zh]/common.json. Updated the locale text found in both AlarmConfigModal.tsx and MerchantConfigModal.tsx to reflect this change
  • Updated the tables to immediately switch between 12/24 hour formats when requested from the user
    • Affected fields: Last-Updated, Current Timeframe (when the merchant is currently active), Next Spawn

Does this introduce a breaking change?

  • Yes
  • No

This PR aims to change how time is presented to the user. No impact to operations are expected.

Other information

Modal Changes

Comparison of the current live models
CurrentModal

Comparison of the changed models in this PR
ModifiedModal

12/24 Hour Changes

Affected areas when configured to 12 hour format
12hrSetting

Affected areas when configured to 24 hour format
24hrSetting

@vercel
Copy link

vercel bot commented May 6, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
lostarktimer ✅ Ready (Inspect) Visit Preview May 6, 2022 at 6:30AM (UTC)

@cwjoshuak cwjoshuak merged commit 8c15dc6 into cwjoshuak:main May 7, 2022
@alairon alairon deleted the 24hr-merchants branch May 23, 2022 04:36
@alairon alairon restored the 24hr-merchants branch May 23, 2022 04: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.

None yet

2 participants