You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by GitBox <gi...@apache.org> on 2020/04/25 18:42:49 UTC

[GitHub] [couchdb-fauxton] popojargo opened a new pull request #1265: Replace notification bar by notification toasts

popojargo opened a new pull request #1265:
URL: https://github.com/apache/couchdb-fauxton/pull/1265


   <!-- Thank you for your contribution!
   
        Please file this form by replacing the Markdown comments
        with your text. If a section needs no action - remove it.
   
        Also remember, that CouchDB uses the Review-Then-Commit (RTC) model
        of code collaboration. Positive feedback is represented +1 from committers
        and negative is a -1. The -1 also means veto, and needs to be addressed
        to proceed. Once there are no objections, the PR can be merged by a
        CouchDB committer.
   
        See: http://couchdb.apache.org/bylaws.html#decisions for more info. -->
   
   ## Overview
   
   <!-- Please give a short brief for the pull request,
        what problem it solves or how it makes things better. -->
   
   
   Replace the existing notification bar by toasts. It takes less place on the UI and it's less distracting. 
   
   ## Testing recommendations
   
   <!-- Describe how we can test your changes.
        Does it provides any behaviour that the end users
        could notice? -->
   
   ### Testing the clear feature: 
   1. Try to create a database with an invalid name (eg: !"/!"/)
   2. The info should appear and slowly disappear after the error shows up
   
   ### Testing the keyboard shortcut:
   Pressing "ESC" when a toast is shown should instantly remove it.
   
   ### Testing toast hover
   By default, toast are a bit transparent. When you hover it, it should be fully opaque.
   
   
   ## GitHub issue number
   
   <!-- If this is a significant change, please file a separate issue at:
        https://github.com/apache/couchdb-fauxton/issues
        and include the number here and in commit message(s) using
        syntax like "Fixes #472" or "Fixes apache/couchdb#472".  -->
   
   #1262 
   
   ## Related Pull Requests
   
   <!-- If your changes affects multiple components in different
        repositories please put links to those pull requests here.  -->
   
   ## Checklist
   
   - [ ] Code is written and works correctly;
   - [ ] Changes are covered by tests;
   - [ ] Documentation reflects the changes;
   - [ ] Update [rebar.config.script](https://github.com/apache/couchdb/blob/master/rebar.config.script) with the correct tag once a new Fauxton release is made
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb-fauxton] popojargo commented on pull request #1265: Replace notification bar by notification toasts

Posted by GitBox <gi...@apache.org>.
popojargo commented on pull request #1265:
URL: https://github.com/apache/couchdb-fauxton/pull/1265#issuecomment-620945122


   @wohali I'll keep digging


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb-fauxton] popojargo commented on pull request #1265: Replace notification bar by notification toasts

Posted by GitBox <gi...@apache.org>.
popojargo commented on pull request #1265:
URL: https://github.com/apache/couchdb-fauxton/pull/1265#issuecomment-619446066


   Will update once #1266  will be merged


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb-fauxton] popojargo commented on pull request #1265: Replace notification bar by notification toasts

Posted by GitBox <gi...@apache.org>.
popojargo commented on pull request #1265:
URL: https://github.com/apache/couchdb-fauxton/pull/1265#issuecomment-620921024


   @wohali Something is going wrong on 2.3.1 but I can't reproduce that locally. Can we merge that and I'll keep working on the E2E tests to make them more robust? 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb-fauxton] Antonio-Maranhao edited a comment on pull request #1265: Replace notification bar by notification toasts

Posted by GitBox <gi...@apache.org>.
Antonio-Maranhao edited a comment on pull request #1265:
URL: https://github.com/apache/couchdb-fauxton/pull/1265#issuecomment-619689297


   @popojargo you're definitely on the right track here - I love the ESC to dismiss it !!!
   I find the colored background doesn't really match the rest of Fauxton's design though. I'd prefer something closer to the original notification bar:
   
   ![image](https://user-images.githubusercontent.com/30349380/80330207-90e5f580-8812-11ea-9ab2-8d1ab56cefce.png)
   
   which you can get with: 
   ```
   .Toastify__toast--info {
     &, .Toastify__close-button--info {
       border: 1px solid #999;
       border-left: 6px solid @infoAlertBackground;
       border-right: 0;
       background-color: #f1f1f1;
       color: black;
     }
   }
   ```
   
   Also, my preference is for `opacity: 1` always (i.e. no transparency), otherwise it's harder read the notification text when the background already has some text/elements. E.g.:
   
   ![image](https://user-images.githubusercontent.com/30349380/80330855-61d08380-8814-11ea-8a0d-9d1ae08707ca.png)
   
   
   ![image](https://user-images.githubusercontent.com/30349380/80330864-69902800-8814-11ea-85d5-033760064c3a.png)
   
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb-fauxton] wohali commented on pull request #1265: Replace notification bar by notification toasts

Posted by GitBox <gi...@apache.org>.
wohali commented on pull request #1265:
URL: https://github.com/apache/couchdb-fauxton/pull/1265#issuecomment-620174410


   @popojargo you can do it yourself - click on Details, go to Travis, log in to Travis using your GH creds, then restart the build... sorry am running out the door, back in 2 hours


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb-fauxton] wohali commented on pull request #1265: Replace notification bar by notification toasts

Posted by GitBox <gi...@apache.org>.
wohali commented on pull request #1265:
URL: https://github.com/apache/couchdb-fauxton/pull/1265#issuecomment-620712399


   @popojargo Hey, this (and #1271) are blocking CouchDB 3.0.1/3.1.0 - it's the very last thing to go in before release, everything else is ready to go. Do you have time to help with this today?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb-fauxton] Antonio-Maranhao commented on pull request #1265: Replace notification bar by notification toasts

Posted by GitBox <gi...@apache.org>.
Antonio-Maranhao commented on pull request #1265:
URL: https://github.com/apache/couchdb-fauxton/pull/1265#issuecomment-619690221


   @popojargo oh.... and thanks for all the fixes/PRs :)


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb-fauxton] wohali commented on pull request #1265: Replace notification bar by notification toasts

Posted by GitBox <gi...@apache.org>.
wohali commented on pull request #1265:
URL: https://github.com/apache/couchdb-fauxton/pull/1265#issuecomment-620941226


   @popojargo I can't merge unless the tests pass.
   
   You can disable the CI check in `.travis.yml` against 2.3.1 for now if you want, but we'll need to fix Fauxton for 2.x if we intend to do any patch releases (2.3.2, 2.3.3, etc.)


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb-fauxton] wohali commented on pull request #1265: Replace notification bar by notification toasts

Posted by GitBox <gi...@apache.org>.
wohali commented on pull request #1265:
URL: https://github.com/apache/couchdb-fauxton/pull/1265#issuecomment-620045115


   I don't have an easy way to test this myself, so I'll just go on what others have said + the images submitted previously.
   
   First, Thanks for not having the new element overlay the toolbar at the top! This was my biggest complaint about the old notifications.
   
   For now please match the old colours in this PR. We can have a separate bike-shed PR to refresh the visual design and bring in other contributors who have helped in the past on this.
   
   I also agree with Antonio: drop the transparency, opaque is best here.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb-fauxton] popojargo commented on pull request #1265: Replace notification bar by notification toasts

Posted by GitBox <gi...@apache.org>.
popojargo commented on pull request #1265:
URL: https://github.com/apache/couchdb-fauxton/pull/1265#issuecomment-620728421


   @wohali Fixing the "flaky" tests since this morning. Should be done by the end of the day


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb-fauxton] popojargo commented on pull request #1265: Replace notification bar by notification toasts

Posted by GitBox <gi...@apache.org>.
popojargo commented on pull request #1265:
URL: https://github.com/apache/couchdb-fauxton/pull/1265#issuecomment-620137786


   I've used a white for the toasts background so it stands out a bit more. I've also moved the toast completely to the right but kept the vertical space with the app bar.
   
   https://share.getcloudapp.com/eDu6PeQK


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb-fauxton] Antonio-Maranhao commented on pull request #1265: Replace notification bar by notification toasts

Posted by GitBox <gi...@apache.org>.
Antonio-Maranhao commented on pull request #1265:
URL: https://github.com/apache/couchdb-fauxton/pull/1265#issuecomment-619689297


   @popojargo you're definitely on the right track here - I love the ESC to dismiss it !!!
   I find the colored background doesn't really match the rest of Fauxton's design. I'd prefer something closer to the original notification bar:
   
   ![image](https://user-images.githubusercontent.com/30349380/80330207-90e5f580-8812-11ea-9ab2-8d1ab56cefce.png)
   
   which you can get with: 
   ```
   .Toastify__toast--info {
     &, .Toastify__close-button--info {
       border: 1px solid #999;
       border-left: 6px solid @infoAlertBackground;
       border-right: 0;
       background-color: #f1f1f1;
       color: black;
     }
   }
   ```
   
   Also, my preference is for `opacity: 1` always (i.e. no transparency), otherwise it's harder read the notification text when the background already has some text/elements. E.g.:
   
   ![image](https://user-images.githubusercontent.com/30349380/80330855-61d08380-8814-11ea-8a0d-9d1ae08707ca.png)
   
   
   ![image](https://user-images.githubusercontent.com/30349380/80330864-69902800-8814-11ea-85d5-033760064c3a.png)
   
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb-fauxton] wohali commented on pull request #1265: Replace notification bar by notification toasts

Posted by GitBox <gi...@apache.org>.
wohali commented on pull request #1265:
URL: https://github.com/apache/couchdb-fauxton/pull/1265#issuecomment-621506471


   Looks like it's passing and you have a +1.... merge it! :)


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb-fauxton] popojargo commented on pull request #1265: Replace notification bar by notification toasts

Posted by GitBox <gi...@apache.org>.
popojargo commented on pull request #1265:
URL: https://github.com/apache/couchdb-fauxton/pull/1265#issuecomment-620173397


   @wohali Mind to rerun Travis please ? :)


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org