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

[DTRA] Maryia/DTRA-739/Add tooltips for Take profit & Stop loss in ContractUpdateForm #13103

Conversation

maryia-deriv
Copy link
Contributor

@maryia-deriv maryia-deriv commented Jan 23, 2024

Changes:

  1. Adding information tooltips for Take profit' & 'Stop loss' in ContractUpdateForm for desktop & mobile (both Trading page and Contract details page), and aligning styles of the form with the latest design.
  2. Fix: 'Apply' button is enabled unnecessarily - initially and when nothing has been changed - instead of only when values are getting updated.
  3. Fix: 'Total profit/loss' in the mobile form is getting updated only when a user interacts with the form instead of updating unstoppably.
  4. Fix: Styles of currency (USD) are broken inside inputs of Take profit & Stop loss.
Also:
  1. Unification of 'Take profit' text for all trade types everywhere including Trade params section,
  2. Cleaning up unused connectWithContractUpdate function,
  3. Spreading passthrough props in all ContractCards and ToggleCardDialog,
  4. Improving Input component by using Text component instead of CSS font styles.

Screenshots:

Before:

After:

Copy link

vercel bot commented Jan 23, 2024

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

Name Status Preview Updated (UTC)
deriv-app ✅ Ready (Inspect) Visit Preview Feb 6, 2024 11:16am

@maryia-deriv maryia-deriv changed the title [DTRA] Maryia/DTRA-739/Add tooltips for Take profit & Stop loss in ContractUpdateForm [DTRA] Maryia/DTRA-739/Add tooltips for Take profit & Stop loss in ContractUpdateForm Jan 23, 2024
Copy link
Contributor

github-actions bot commented Jan 23, 2024

A production App ID was automatically generated for this PR. (log)

Click here to copy & paste above information.
- **PR**: [https://github.com/binary-com/deriv-app/pull/13103](https://github.com/binary-com/deriv-app/pull/13103)
- **URLs**:
    - **w/ App ID + Server**: https://deriv-app-git-fork-maryia-deriv-maryia-dtra-739tooltips-76e4cc.binary.sx?qa_server=red.derivws.com&app_id=24255
    - **Original**: https://deriv-app-git-fork-maryia-deriv-maryia-dtra-739tooltips-76e4cc.binary.sx
- **App ID**: `24255`

Copy link
Contributor

github-actions bot commented Jan 23, 2024

🚨 Lighthouse report for the changes in this PR:

Category Score
🔺 Performance 28
🟧 Accessibility 89
🟢 Best practices 92
🟧 SEO 85
🟧 PWA 78

Lighthouse ran with https://deriv-app-git-fork-maryia-deriv-maryia-dtra-739tooltips-76e4cc.binary.sx/

@coveralls
Copy link

coveralls commented Jan 24, 2024

Coverage Status

Changes unknown
when pulling 2011e19 on maryia-deriv:maryia/DTRA-739/tooltips-take-profit-stop-loss
into ** on binary-com:master**.

tooltip_label={tooltip_text}
tooltip_label={
<Localize i18n_default_text='When your profit reaches or exceeds this amount, your trade will be closed automatically.' />
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unification of 'Take profit' text for all trade types everywhere including this one displayed in Trade params section.

Copy link

sonarqubecloud bot commented Feb 6, 2024

Quality Gate Passed Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

2 New issues
0 Security Hotspots
No data about Coverage
3.7% Duplication on New Code

See analysis details on SonarCloud

Copy link
Contributor

github-actions bot commented Feb 6, 2024

Generating Lighthouse report...

@balakrishna-deriv balakrishna-deriv merged commit 7b59c31 into deriv-com:master Feb 7, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants