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

Dashboard Notifications: Issues noted by reviewers #1223

Open
birdofpreyru opened this issue Jul 27, 2017 · 0 comments
Open

Dashboard Notifications: Issues noted by reviewers #1223

birdofpreyru opened this issue Jul 27, 2017 · 0 comments

Comments

@birdofpreyru
Copy link
Collaborator

  • banners config is placed under "/app/my-dashboard/notifications/notifications.jade" - This is confusing, there should
    be a separate banners folder under my-dashboard which should contain banners related code configs.

  • app\directives\notification\notification.directive.js
    empty function

    function activate(){
    
    }
    
  • After an all amazing last yera TCO16 Beijing Regionals we were very excited to go back and do an event in…
    You should be possible to show full description when user hover over to trimmed text area of notification area

  • "headerExpand": "Expanded header worked",
    "detailsExpand": "Expanded details worked",
    is actually not worked or it will use
    "header": "Another notification demo",
    "details": "Without video but has a live flag",
    for expand area
    see notification.jade
    .expanded(ng-show="vm.expanded", ng-style="{'background-image': 'url('+vm.loadImage(config.backgroundImage)+')'}")
    .mask
    .left(ng-style="{'height': (config.height || vm.defaultHeight) + 'px'}")
    .header {{config.header}}
    .details {{config.details | characters:110}}

  • app\directives\notification\notification.jade
    .details {{config.details | characters:110}}
    should define from constant configuration for 110 or max length to show

  • background Image you should be possible to use image url directly
    actually this is image url used in current production banner
    https://s3.amazonaws.com/app.topcoder.com/61a6ddc9638a40beff7adfd5c00d13c0.jpg

  • see wiki

    1. the message to show when the block is collapsed (separately the header and the details);
    2. the message to show when the block is expanded (separately the header and the details, if not given, should default to (1));
      while you will actually always show (1) for expanded text
  • "2) the message to show when the block is expanded (separately the header and the details, if not given, should default to (1));"
    This is not properly implemented, see https://drive.google.com/open?id=0B1ZT_oEfOX1wYkd1SDU0WmR4eW8
    "Another notification demo"/"Without video but has a live flag" are still shown even if the notification was expanded.
    In such case it should show the configured values for exapaned header and details :
    "Expanded header worked"/"Expanded details worked"

  • The facebook video share button is hard to click/work anymore it will always close banner actually

  • If I add long text for "header" directly
    {
    "header": "SRM 789 is starting in 10 dasdasd ashkjas fasjk fask fka hahs fkas fklasjf klasj gk asjgkas gklsajlkf jasklg jasklgjklas jglkas jgkasjglkas jgklas gklasjgkl asjklg jalks gjaskl jgklasgjklas gjklas jgklas gjklasj gkl asjkgl asjklgj asklgj kasl jgklasjgkl sajglkas jgklasjlkg jaskl gjkasl jgkasjglkas jgklas jgklasjgklasjgklasjgkl jasklgjaskl gjkl asjgk lasjgkla sgjk lasgjs fkas fjkas fays (June 3, 22:30pm)",
    "details": "Registration will be ope n on the ashfkas fasday of the competition. Don't forget to join us, add it to your calendar",
    "headerExpand": "TCO17 Beijing Regiona haskfkas fkas gklas hgkas gklas ghkasg kasl gkasl glkasjg lkasjgkl as kgljaslkg jasklgls",
    "detailsExpand": "After an all amazing las akf kalsjgklas gklasjgklas jgklajs gst yera TCO16 Beijing Regionals we were very excited to go back and do an event in some how other text",
    "backgroundImage": "tco17-beijing-banner.jpg",
    "redirectTo": "https://tco17.topcoder.com/",
    "redirectText": "Learn More",
    "videoCode": "https://www.youtube.com/embed/fTdov0zyFu8?autoplay=1",
    "live": false,
    "liveExpand": false,
    "height": 480
    }
    the layout will break and it will not trim text like other text fields(although it will work well if I add to details)
    see
    long header1.png
    long header2.png

  • Although it is not normal
    redirectText will not work well with long text

  • I try to add news.json with such data
    {
    "backgroundImage": "tco17-beijing-banner.jpg",
    "live": false,
    "liveExpand": false
    }
    It will show empty header with extra :

  • Test under Firefox 51 Ubuntu 16.04
    The first notification area Play Video icon will not show in center of right part area
    There is no such issue for Chrome 59 in same computer or second notification area under Firefox
    see
    play icon not in center under firefox .png

  • similar issue for IE11 under windows 8.1
    see play icon not in center under IE11 .png

  • see animate.mp4 from design B
    it should exist animate if Collapse/ Expand notificatio

  • You should be better to custom notification Collapsed banner color

  • The play video text will display too close to bottom for small width device under Firefox 51 under Ubuntu 16.04 or windows 8.1
    Chrome under Ubuntu has no such similar problem actually it will look longer
    see
    Play Video too close to bottom under Firefox.png

  • Mobile layout will break under IE11 windows 8.1
    issues happened for banner/notifications at same time
    see
    Mobile layout break under IE11.png

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

No branches or pull requests

1 participant