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

fix: prevent campaign names from causing dashboard rows to overflow #1147

Merged
merged 3 commits into from
Apr 26, 2021

Conversation

zwliew
Copy link
Contributor

@zwliew zwliew commented Apr 22, 2021

Problem

This is a minor design issue I noticed during testing:

A particularly long campaign name causes the dashboard table to overflow its container. This doesn't look very good.

Solution

Improvements:

  • Only prepend Copy of to campaign names which don't already have it (explanation in commit 6a3db9a)

Bug Fixes:

  • Prevent the dashboard table from overflowing its container

Before & After Screenshots

BEFORE:

Before screenshots

Screenshot 2021-04-15 at 2 19 05 PM

Screenshot 2021-04-15 at 2 19 22 PM

AFTER:

After screenshots

Screenshot 2021-04-15 at 2 20 04 PM

Screenshot 2021-04-15 at 2 19 45 PM

Tests

Duplicating a duplicated campaign should not prepend Copy of to the campaign's name

  1. Try duplicating a campaign which already has Copy of at the start of its name.
  2. Assert that the campaign duplication modal does not prepend yet another Copy of to the new campaign's name.

Table should not overflow the container

  1. Create a campaign with a very long name.
  2. Assert that the dashboard table does not have a horizontal scrollbar.

Deploy Notes

No deploy notes.

Todo

  • Maybe add a unit test for ensuring the Copy of prepending functionality works as intended?

zwliew and others added 3 commits April 21, 2021 11:11
If a user duplicates a campaign, then duplicates the duplicate, and does
it over and over again, the resulting campaign name becomes something
like "Copy of Copy of Copy of Copy of ..."

This is pretty ridiculous, and doesn't really make sense to have.
All these campaigns are really just similar copies of the original campaign anyway.

Hence, only prepend "Copy of " if the campaign name doesn't already have
it.
Copy link
Contributor

@lamkeewei lamkeewei left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this. I've also added another fix to add ellipsis for long campaign names in TitleBar.

image

@lamkeewei lamkeewei merged commit 808cce4 into develop Apr 26, 2021
@lamkeewei lamkeewei deleted the fix-overflowing-dashboard-row branch April 26, 2021 03:41
lamkeewei added a commit that referenced this pull request May 5, 2021
* develop:
  refactor: use shared function to initialize models (#1172)
  chore: setup scaffolding for backend tests (#940)
  1.23.0
  fix(frontend): fix frontend test flakiness (#1162)
  fix: update successful delivery status only if error does not exist (#1150)
  chore: upgrade dependencies (#1153)
  feat: add unit tests for error states in critical workflows (#1118)
  feat: support whitelisting domains through `agencies` table (#1141)
  feat: add tests for happy paths in critical workflows (#1110)
  fix: prevent campaign names from causing dashboard rows to overflow (#1147)
  fix(email): Fix SendGrid fallback integration (#1026)
lamkeewei added a commit that referenced this pull request May 5, 2021
* develop:
  feat: refactor msg template components; add telegram character limit (#1148)
  refactor: use shared function to initialize models (#1172)
  chore: setup scaffolding for backend tests (#940)
  1.23.0
  fix(frontend): fix frontend test flakiness (#1162)
  fix: update successful delivery status only if error does not exist (#1150)
  chore: upgrade dependencies (#1153)
  feat: add unit tests for error states in critical workflows (#1118)
  feat: support whitelisting domains through `agencies` table (#1141)
  feat: add tests for happy paths in critical workflows (#1110)
  fix: prevent campaign names from causing dashboard rows to overflow (#1147)
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.

2 participants