Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Xiaohua Guan created an issue. See original summary.

Xiaohua Guan’s picture

Xiaohua Guan’s picture

FileSize
21.2 KB
Xiaohua Guan’s picture

FileSize
21.2 KB
Xiaohua Guan’s picture

Status: Active » Needs review

@yas

Please review the patch file. Thanks.

yas’s picture

yas’s picture

yas’s picture

Issue summary: View changes

@xiaohua-guan

Thank you for the patch. It looks great. Here is the screenshot for us-east-1 and ap-northeast-1.

us-east-1
us-east-1

ap-northeast-1
ap-northeast-1

I want to request the following minor cosmetic changes:

  1. Add one white space in between e.g. Hourly and ($) on the header. Therefore it will become Hourly ($), Daily ($), Monthly ($), 1 Year ($), 3 Year ($).
  2. The each header element should be aligned to be centered (not left-justified)
  3. At On-demand Hourly ($), the floating number digits should be four (e.g. 1.2345 or 4.5678 (rounded value) for micro instance types.
  4. At On-demand Daily ($) and On-demand Monthly ($), the floating number digits should be two (e.g. 1.23 or 4.56 (rounded value)
  5. At On-demand Yearly ($), the number can be an integer (rounded value) because it is easy to compare to RI 1 Year ($).
  6. Add an action link for Instance Type Prices into Server Template's Add / Edit page.
Xiaohua Guan’s picture

FileSize
22.27 KB
Xiaohua Guan’s picture

@yas

I fixed the code. Please review the latest patch file. Thanks.

yas’s picture

@xiaohua-guan

Thank you for your update. It looks good to me, and I would like to have the following further request to make it perfect:

  1. Add vertical-align: middle; to all table elements (headers and rows)
  2. Change the one year period as follows:
    ...
            const ONE_YEAR = 365.25
    ...
            'on_demand_monthly' => floatval($hourly_rate) * 24 * ONE_YEAR / 12,
            'on_demand_yearly'  => floatval($hourly_rate) * 24 * ONE_YEAR,
    

@baldwinlouie

What do you think?

Xiaohua Guan’s picture

FileSize
22.44 KB
Xiaohua Guan’s picture

@yas

Thanks for your comment. I fixed the code as you said. Please take a look again. Thanks.

yas’s picture

@xiaohua-guan,

Thank you for the update patch. It looks perfect to me but sorry, can you "Add an action link for Instance Type Prices into Server Template's Add / Edit page." for a new browser window?

baldwinlouie’s picture

@yas and @xiaohua-guan, Code looks good to me.

Xiaohua Guan’s picture

Xiaohua Guan’s picture

@yas

> can you "Add an action link for Instance Type Prices into Server Template's Add / Edit page." for a new browser window?

Yes, I make the link to open as a new window.

Please review the patch file. Thanks.

yas’s picture

Status: Needs review » Reviewed & tested by the community

@xiaohua-guan

Great, thank you for the update! I'll merge the patch and close the issue as Fixed. Thank you for your efforts.

  • yas committed f998f80 on 8.x-1.x authored by Xiaohua Guan
    Issue #3036287 by Xiaohua Guan, yas, baldwinlouie: Add EC2 instance...
yas’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.