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

Hide ROI by default #1468

Merged
merged 13 commits into from
Sep 6, 2024
Merged

Hide ROI by default #1468

merged 13 commits into from
Sep 6, 2024

Conversation

tiagofilipenunes
Copy link
Collaborator

@tiagofilipenunes tiagofilipenunes commented Sep 4, 2024

  • Create optional config param
  • If param is false or undefined, hide StrategyBlockRoi and adjust StrategyBlockBudget to the full column width
  • Update readme with new flag

Note: ROI block is hidden by default

fix #1467

- Create optional config param
- If param is false or undefined, hide StrategyBlockRoi and adjust StrategyBlockBudget to the full column width
- Update readme with new flag
Copy link

cloudflare-workers-and-pages bot commented Sep 4, 2024

Deploying carbon-app-sei with  Cloudflare Pages  Cloudflare Pages

Latest commit: f3ee971
Status: ✅  Deploy successful!
Preview URL: https://27a8a192.carbon-app-sei.pages.dev
Branch Preview URL: https://issue--1467.carbon-app-sei.pages.dev

View logs

Copy link

cloudflare-workers-and-pages bot commented Sep 4, 2024

Deploying carbon-app with  Cloudflare Pages  Cloudflare Pages

Latest commit: f3ee971
Status: ✅  Deploy successful!
Preview URL: https://dcfc59e6.carbon-app-csq.pages.dev
Branch Preview URL: https://issue--1467.carbon-app-csq.pages.dev

View logs

Copy link

cloudflare-workers-and-pages bot commented Sep 4, 2024

Deploying carbon-app-celo with  Cloudflare Pages  Cloudflare Pages

Latest commit: f3ee971
Status: ✅  Deploy successful!
Preview URL: https://9b26665e.carbon-app-celo.pages.dev
Branch Preview URL: https://issue--1467.carbon-app-celo.pages.dev

View logs

Copy link

cloudflare-workers-and-pages bot commented Sep 4, 2024

Deploying carbon-app-blast with  Cloudflare Pages  Cloudflare Pages

Latest commit: f3ee971
Status: ✅  Deploy successful!
Preview URL: https://5e95e91a.carbon-app-blast.pages.dev
Branch Preview URL: https://issue--1467.carbon-app-blast.pages.dev

View logs

@ashachaf
Copy link
Collaborator

ashachaf commented Sep 4, 2024

  • sort by ROI should also be hidden
  • explore / my strategies filter option should not expose roi
  • IF hidden, use sort by total budget ascending
image
  • strategy page - apply same rule
image

@ashachaf
Copy link
Collaborator

ashachaf commented Sep 5, 2024

  • updated default sort is missing the checkmark indication in the menu
image

- Add comment explaining roi flag
- Set strategySort after definition and change getSortAndFilterItems to filter out empty string titles
@tiagofilipenunes tiagofilipenunes merged commit 12a6897 into main Sep 6, 2024
4 checks passed
@tiagofilipenunes tiagofilipenunes deleted the issue-#1467 branch September 6, 2024 15:14
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.

Overview card - remove ROI
3 participants