Alert improvement and adding byebug gem #383
Conversation
|
Success alerts are visible now:
|
|
Feel free to merge this when you're ready! It looks great! |
|
@sashadev-sky can you try this PR on your system? |
|
@gauravano yes will do that today! Sorry just saw this |
|
No issue @sashadev-sky! |
|
Actually, don't review it now Sasha. I am thinking of doing some more minor fixes in this PR only. |
There was a problem hiding this comment.
This looks great! But there is an issue. The UI has recently been updated to look like this:
I rebased your branch and ran it again and your alert comes out looking like this:
I think before we merge this we have to fix the navigation bar first. I don't have any problem doing that and can get that done very quickly with your alert in mind on top of your PR if you would like! Or you are free to do it. As long as @jywarren agrees we maybe should fix it up a bit.
I have few issues with the UI update:
- I think the grey worked with the pages color palette, and the mapknitter logo is nice and should be kept. More importantly it is too big and now overlapping the map, and the search bar is in a strange location
- Some of the changes I do like, though, I think the layout of the different sections with page name / logo and the way the words highlight looks sharp and clean 👍
|
@gauravano @jywarren is there something wrong with my view because I see the original PR for the nav bar was not intended to look like this |
|
@gauravano sorry I missed that message! But, I do think the things I wrote should be addressed regardlessly, doesn't have to be in this PR |
|
Oh hmm, maybe @stefannibrasil can look at this as she recently updated the navbar? Maybe there was a merge conflict or something? Thanks! |
|
Hum, let me investigate this. It was not supposed to look like that at all. It looks like it's still using the old CSS configuration... Is it possible? I'm not sure, I'll get back to you when I have something more concrete to say. About the display of the alerts, I think that if they are placed after the navbar, they should be shown correctly. There is a padding-bottom after the navbar, maybe the alert is being placed in the middle or something? |
|
actually, I need some help to get the new changes from this repo. I tried using git fetch and git pull to get the changes after the new navbar was merged, but it does not get the navbar changes. I wanted to test the 'main' branch with the most updated code. How do you all get the changes in your local machine? I forked the repo and added an upstream remote repo to my cloned local repo. I did that on plots2 and it always worked, so I don't know if I'm missing something here specific for MapKnitter. Thanks! |
|
Hmm, let me check. The PR was #360... and it does appear in main branch commits: https://github.com/publiclab/mapknitter/commits/main Are you in your local |
|
I'm in the main but this might be something related to having the master in mind when I was installing the project. I'll try installing it again. Thanks, @jywarren |
|
hi everyone, I was able to get the updates and test it locally :) I added some notes on this PR #399 about the issue with the new navbar. Thanks, @sashadev-sky for pointing the problem with the navbar, Bootstrap 3 has this problem that wasn't aware. Please let me know if it's okay with the changes that I added there, but I think it should fix the problem. |
|
Hi everyone, Thanks @sashadev-sky for the review and @stefannibrasil for the navbar fix. Sorry wasn't able to keep up with this PR. Rebasing my PR and doing some more fixes.. |
grvsachdeva
force-pushed
the
alert_improvement
branch
from
972f947 to
acddda4
Compare
|
@jywarren it's ready, please review it and merge it if you find all changes alright. Thanks! |
|
@jywarren please review it. Thanks! |
|
🎉 Thank you Gaurav!!! |
* byebug gem added and alerts in separate file * adding byebug history to gitignore * adding timestamp to redirect * added z-index to render login dropdown above leaflet icon
This reverts commit 7502b53.
* byebug gem added and alerts in separate file * adding byebug history to gitignore * adding timestamp to redirect * added z-index to render login dropdown above leaflet icon
* byebug gem added and alerts in separate file * adding byebug history to gitignore * adding timestamp to redirect * added z-index to render login dropdown above leaflet icon
* byebug gem added and alerts in separate file * adding byebug history to gitignore * adding timestamp to redirect * added z-index to render login dropdown above leaflet icon
* Shortening docker image in ~30% * Caching bundle, gathering env variables and using newer sintax * Creating startup script and env file * Improving travis CI configuration * Loading assets in production env * Allow uglifier to interpret ES6 * Fix start command * Fix travis script * Tweak travis script * Add delay * Revert assets changes * Return to Mysql5.7 * Tweak travis script * Fix make redeploy-container command * Add db migrate and precompile step. * Add bower install to Makefile * Clean after docker run. Avoid one bower run. * Changes to be able to build container in Google Cloud * Remove spurious symlink * Copy config examples when making build * Export env variable name * Tag cloud image * Add timeout * Push to cloud registry * Fix jenkins build error with docker-compose tty * Add app to container and .dockerignore all else * Copy configuration files when deploying to GCE * Allow copy config to container * Time extended (for cloud build & push) * Delete redundant index.html.erb file (#427) * Setupcoveralls (#438) * Add coveralls * Fix gemfile * Fix env variable * Add coveralls token * Update README.md * Remove legacy image controller code #404 (#417) Deleted the lines from the selection indicated in the issue. * Change comment count on comment creation via AJAX #441 (#443) This closes issue #441 "Change comment count on comment creation via AJAX #441" by incrementing comments-number each time a new comment is added. This would ensure that the counter indicating the number of comments is increased without needing to refresh the page. * update syntax of active record query(license method) (#439) Fixes #437 * Docker improve rebased (#450) * Shortening docker image in ~30% * Caching bundle, gathering env variables and using newer sintax * Creating startup script and env file * Improving travis CI configuration * Loading assets in production env * Allow uglifier to interpret ES6 * Don't dettach when building container in travis * Fix start command * Fix travis script * Try to resolve travis tests invocation * Tweak travis script * Add delay * Bundle install before db setup * Shortening docker image in ~30% * Caching bundle, gathering env variables and using newer sintax * Creating startup script and env file * Improving travis CI configuration * Loading assets in production env * Allow uglifier to interpret ES6 * Fix start command * Fix travis script * Tweak travis script * Add delay * Revert assets changes * Return to Mysql5.7 * Tweak travis script * Fix make redeploy-container command * Add db migrate and precompile step. * Add bower install to Makefile * Clean after docker run. Avoid one bower run. * updte pr template (#448) * Bump recaptcha from 4.13.1 to 4.13.2 (#452) Bumps [recaptcha](https://github.com/ambethia/recaptcha) from 4.13.1 to 4.13.2. - [Release notes](https://github.com/ambethia/recaptcha/releases) - [Changelog](https://github.com/ambethia/recaptcha/blob/master/CHANGELOG.md) - [Commits](ambethia/recaptcha@v4.13.1...v4.13.2) Signed-off-by: dependabot[bot] <support@dependabot.com> * Restructure rake test task runner (#380) * add a mysql setup file * Squash commits * Update README.md (#456) * Change run to exec (#457) * Bump paperclip from 4.2.4 to 4.3.7 (#285) Bumps [paperclip](https://github.com/thoughtbot/paperclip) from 4.2.4 to 4.3.7. - [Release notes](https://github.com/thoughtbot/paperclip/releases) - [Changelog](https://github.com/thoughtbot/paperclip/blob/v4.3.7/NEWS) - [Commits](thoughtbot/paperclip@v4.2.4...v4.3.7) Signed-off-by: dependabot[bot] <support@dependabot.com> * Bump test-unit from 3.3.0 to 3.3.1 (#458) Bumps [test-unit](https://github.com/test-unit/test-unit) from 3.3.0 to 3.3.1. - [Release notes](https://github.com/test-unit/test-unit/releases) - [Commits](test-unit/test-unit@3.3.0...3.3.1) Signed-off-by: dependabot[bot] <support@dependabot.com> * Bump coveralls from 0.7.1 to 0.8.22 (#453) Bumps [coveralls](https://coveralls.io) from 0.7.1 to 0.8.22. Signed-off-by: dependabot[bot] <support@dependabot.com> * gridview aligned (#464) * Alert improvement and adding byebug gem (#383) * byebug gem added and alerts in separate file * adding byebug history to gitignore * adding timestamp to redirect * added z-index to render login dropdown above leaflet icon * fixed image partial rendering when no images (#423) * fixed image partial rendering when no images. * toggle no images <p> om upload * fixed image partial rendering when no images. * Bump recaptcha from 4.13.2 to 4.14.0 (#471) Bumps [recaptcha](https://github.com/ambethia/recaptcha) from 4.13.2 to 4.14.0. - [Release notes](https://github.com/ambethia/recaptcha/releases) - [Changelog](https://github.com/ambethia/recaptcha/blob/master/CHANGELOG.md) - [Commits](ambethia/recaptcha@v4.13.2...v4.14.0) Signed-off-by: dependabot[bot] <support@dependabot.com> * add a flash error when adding tags and not logged in (#473) * Upgrade app to Bootstrap 4 (#480) * Bootstrap 4 small button fixes (#488) * Add tests for comments and maps (#467) * Updated query style (#436) (#469) * Dynamic ports (#462) * Dynamic port in compose file * Omit setting container name * Add initial sql dump entry * Avoid resetting database on build * Shortening docker image in ~30% * Caching bundle, gathering env variables and using newer sintax * Creating startup script and env file * Improving travis CI configuration * Loading assets in production env * Tweak travis script * Roll back to using Debian 9 with custom built GDAL (#489) * Switch back to Debian 9 Stretch * Simplify docker image * Bump Ruby to 2.4.6 * Re-add dependency * Add dependency (zip) * Try pip install gdal * Install libgdal-dev * Revert attept to use pip * Bump ruby * Avoid naming containers in compose file * Avoid overwriting database on redeploy-container * Allow to load mysql dump * Include own GDAL packages * Disable ipv6 to prevent error * Add missing Amazon S3 yml to Makefile * Document unstable instance * Changes to be able to build container in Google Cloud * Copy config examples when making build * Add app to container and .dockerignore all else * Fixed missed merge * Add db configurability by env vars for containers * Fix db config * Copy configs * Switch keyserver * Add env vars, tweak make * Substitute env vars parameters * Env var control * Show env vars * env not ENV * add DB_SOCKET * Add recomended parameters * Not deploy app engine, show cloudsql dir * Omit list cloudsql dir * Added correct image tag * Add database parameters as env vars * Support $PORT env var * Using Node 12 and Yarn for Dockerfile.txt as well * Changing Passenger's port on production env * Setting local db for travis * set .env PORT to $PORT * Remove .env * Compose environment variableZ fallback * Revert all files under /app to versions in main * Revert to main * Delete unneeded files * Remove extra files from rebase * Add bundle install as build step * Deleted not needed Dockerfile * Missed RUN in Dockerfile * Add precompile step * Hardcode environment at build time * Adding missing yaml and update bootsnap version * Omit /app/tmp from volume * Revert try to get precompile to work * Clean up patch for merging * List variables in app.yaml * Tweak for jenkins * Add .env for jenkins * Fix PORT for jenkins/docker-compose * Address PORT properly * New form ports * Enclose docker-compose ports in quotes * ports yaml should be object not array * Try different format for ports * Try docker-compose format * Redirect script for AppEngine * Try to revertt to working condition for appengine * Point PORT in Procfile * Revert to known good config in appengine * Tweak assets precompilation * Restore PORT setting * Add redirect to map /warps directory to legacy archive * Add .env for jenkins/docker-compose * Add hardcoded route to legacy warps * Remove .env for appengine * Satisfy appengine docker-compose * Ignore app.yaml * Ignore app.yaml secrets * Satisfy Jenkins wihout hurting appengine hopefully Co-authored-by: Sebastian Silva <sebastian@fuentelibre.org> Co-authored-by: rarrunategu1 <parker.rose@ymail.com> Co-authored-by: Jeffrey Warren <jeff@unterbahn.com> Co-authored-by: Milo MacPhail <40954168+milomacphail@users.noreply.github.com> Co-authored-by: Sonali Agrawal <sonali9696@gmail.com> Co-authored-by: Ananya Agrawal <33188930+ananya@users.noreply.github.com> Co-authored-by: dependabot[bot] <dependabot[bot]@users.noreply.github.com> Co-authored-by: Sasha Boginsky <41092741+sashadev-sky@users.noreply.github.com> Co-authored-by: Kaustubh Nair <kaustubh.nair@iiitb.org> Co-authored-by: Divya Baid <32747809+divyabaid16@users.noreply.github.com> Co-authored-by: Gaurav Sachdeva <sachdeva.gaurav1997@gmail.com> Co-authored-by: Govind Jeevan <govindjeevan7@gmail.com> Co-authored-by: Cess <cessmbuguar@gmail.com> Co-authored-by: Stefanni <stefannibrasil@gmail.com> Co-authored-by: hc-barker <hc-barker@users.noreply.github.com>
Issues solved:
Make sure these boxes are checked before your pull request is ready to be reviewed and merged. Thanks!
rake testThanks!