feat: Login Box: Show info message when page directs to login page #5531

Draco9421 · 2020-11-08T18:16:27Z

Short description of what this resolves:

Changes proposed in this pull request:

Checklist

I have read the Contribution & Best practices Guide.
My branch is up-to-date with the Upstream development branch.
The acceptance, integration, unit tests and linter pass locally with my changes
I have added tests that prove my fix is effective or that my feature works
I have added necessary documentation (if appropriate)

vercel · 2020-11-08T18:16:32Z

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/eventyay/open-event-frontend/18d6h6alu
✅ Preview: https://open-event-frontend-git-development.eventyay.vercel.app

iamareebjamal · 2020-11-08T23:14:55Z

tests/helpers/flash-message.js

@@ -0,0 +1,3 @@
+import FlashObject from 'ember-cli-flash/flash/object';
+
+FlashObject.reopen({ init() {} });


Again, why is this needed?

iamareebjamal · 2020-11-08T23:15:47Z

app/templates/application.hbs

+
+     <div class="flash-messages ui container">
+      {{#each flashMessages.queue as |flash|}}
+         <div class="ui negative message">


Flash message can be of any type. Not just danger/negative. Modify UI accordingly

Draco9421 · 2020-11-12T09:50:28Z

@iamareebjamal please review the changes.

iamareebjamal · 2020-11-12T10:23:49Z

Please fix the build before asking for review and check previous reviews as well

Draco9421 · 2020-11-13T12:43:41Z

Can you please tell me how can I built test case file for it.

codecov · 2020-11-17T11:06:12Z

Codecov Report

Merging #5531 (164d846) into development (91fb290) will increase coverage by 0.03%.
The diff coverage is 60.00%.

@@               Coverage Diff               @@
##           development    #5531      +/-   ##
===============================================
+ Coverage        23.45%   23.48%   +0.03%     
===============================================
  Files              512      511       -1     
  Lines             5445     5454       +9     
  Branches            63       63              
===============================================
+ Hits              1277     1281       +4     
- Misses            4152     4157       +5     
  Partials            16       16

Impacted Files	Coverage Δ
app/components/public/call-for-speakers.js	`0.00% <0.00%> (ø)`
app/controllers/public/index.js	`6.25% <0.00%> (-0.10%)`	⬇️
app/routes/create.js	`100.00% <100.00%> (ø)`
app/helpers/general-date.js	`66.66% <0.00%> (-33.34%)`	⬇️
app/helpers/header-date.js

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 91fb290...164d846. Read the comment docs.

iamareebjamal · 2020-11-17T11:14:18Z

https://github.com/fossasia/open-event-frontend/pull/5531/files#r519491485

Draco9421 · 2020-11-17T12:29:07Z

https://github.com/fossasia/open-event-frontend/pull/5531/files#r519491485

I didn't added that it was created automatically when I installed ember-cli-flashmessages

iamareebjamal · 2020-11-17T12:41:33Z

Delete it and see if it is still working

Draco9421 · 2020-11-17T13:09:33Z

Delete it and see if it is still working

Yes sir It's working fine without it

iamareebjamal · 2020-11-17T13:15:06Z

Then please delete it and add other messages

iamareebjamal · 2020-11-17T14:00:21Z

tests/test-helper.js

@@ -3,6 +3,8 @@ import QUnit from 'qunit';
 import config from 'open-event-frontend/config/environment';
 import { setApplication } from '@ember/test-helpers';
 import { start } from 'ember-qunit';
+import './helpers/flash-message';


sorry sir forgot to delete that line.

iamareebjamal · 2020-11-17T14:10:52Z

tests/test-helper.js

@@ -3,6 +3,7 @@ import QUnit from 'qunit';
 import config from 'open-event-frontend/config/environment';
 import { setApplication } from '@ember/test-helpers';
 import { start } from 'ember-qunit';
+


Draco9421 · 2020-11-17T14:12:24Z

Sir there is already a message which is shown when we verify our email address. So should I do any changes on it or not.

iamareebjamal · 2020-11-17T14:37:38Z

No

Draco9421 · 2020-11-18T12:47:08Z

Sir I didn't find any other case which requires any type of flashMessage.

iamareebjamal · 2020-11-18T13:01:13Z

There are several actions which require you to login first

iamareebjamal · 2020-11-25T13:23:20Z

@codedsun @daretobedifferent18 Please test and check if anything is missing

divyamtayal

Please change the styling for this flash box, it does not look good here (either add margin across the message) whatever feels good.

Change at

Add Speaker
Order Tickets

divyamtayal

If possible pls add the info icon as above. so that it matches the styling.

iamareebjamal · 2020-11-25T18:12:01Z

icon not needed

mariobehling · 2020-11-25T19:12:35Z

Please do not use red bold danger message for this kind of notification.

Draco9421 · 2020-11-25T19:17:51Z

Please change the styling for this flash box, it does not look good here (either add margin across the message) whatever feels good.

@daretobedifferent18 @iamareebjamal @codedsun Is this one good enough sir?

iamareebjamal · 2020-11-27T16:34:02Z

Please update screenshots. And @daretobedifferent18 @sachinchauhan2889 Please test

sachinchauhan2889

screenshots

hey, you are working nice.
I think some more changes needed.
flash messages inside modal is not looking great. there are two nested semantic ui messages both of green color. this is not looking great.

i think your previous flash message looks nice then this. look at this

your-previous-modal

@iamareebjamal sir, compare both modal. i think prevoius one was better.

i think @daretobedifferent18 suggest you to add margin around flash message in modal. but you created two nested semantic ui messages.

if you want to add margin around flash message you can create a class in app/styles folder and use that class in outer most div of flash message.

sachinchauhan2889 · 2020-11-27T17:13:06Z

tests/test-helper.js

@@ -3,6 +3,7 @@ import QUnit from 'qunit';
 import config from 'open-event-frontend/config/environment';
 import { setApplication } from '@ember/test-helpers';
 import { start } from 'ember-qunit';
+


please remove this blank line.

sachinchauhan2889 · 2020-11-27T17:55:29Z

app/templates/components/forms/login-form.hbs

+  <div class="flash-messages ui container">
+    {{#each flashMessages.queue as |flash|}}
+        <div class="ui {{flash.type}} message">
+         <div class="header">


@mariobehling sir, suggest not to use bold red color text. you can remove bold text my removing header class.
header class bolded text of semantic ui message.
you can remove header class to get non-bolded text.

ok sir I will remove it.

sachinchauhan2889 · 2020-11-27T17:58:00Z

app/templates/components/modals/login-signup-modal.hbs

+<div class="flash-messages ui container mb-5">
+    {{#each flashMessages.queue as |flash|}}
+        <div class="ui {{flash.type}} message">
+         <div class="header ui inline {{flash.type}} large message">


no need to nested semantic ui messages.

app/templates/components/forms/login-form.hbs

divyamtayal · 2020-11-27T18:46:54Z

app/templates/components/forms/login-form.hbs

@@ -1,4 +1,13 @@
 <div class="ui stackable {{unless this.noSocial 'three' 'one'}} column doubling centered grid">


Add class mt-4 here

divyamtayal · 2020-11-27T18:53:05Z

app/templates/components/modals/login-signup-modal.hbs

@@ -1,3 +1,12 @@
+<div class="flash-messages ui container mb-5">


remove ui container mb-5 and add m-2

and revert the changes as asked by @sachinchauhan2889 about nesting.

sachinchauhan2889

looks nice

sachinchauhan2889 · 2020-11-27T19:51:45Z

app/templates/components/modals/login-signup-modal.hbs

-        <div class="ui {{flash.type}} message">
-         <div class="header ui inline {{flash.type}} large message">
+      <div class="ui {{flash.type}} message">
+       <div class="header">


please remove header class from here also.

divyamtayal · 2020-11-27T20:10:30Z

Pls update the final screenshots

Draco9421 · 2020-11-27T20:16:11Z

Pls update the final screenshots

done sir

sachinchauhan2889

great work @Draco9421 .

Changes looks nice to me.

@iamareebjamal sir, this PR is ready for your review.

Draco9421 · 2020-11-28T12:01:37Z

thank you for merging it

flash message added during login

3af3294

vercel bot deployed to Preview November 8, 2020 18:16 View deployment

iamareebjamal reviewed Nov 8, 2020

View reviewed changes

flash message added during login with changed UI

a78c1f1

vercel bot deployed to Preview November 9, 2020 09:47 View deployment

Draco9421 requested a review from iamareebjamal November 9, 2020 09:48

testing error solved

f292706

vercel bot deployed to Preview November 17, 2020 10:53 View deployment

Delete flash-message.js

d37cffa

vercel bot deployed to Preview November 17, 2020 13:38 View deployment

iamareebjamal reviewed Nov 17, 2020

View reviewed changes

Update test-helper.js

71ec5f8

vercel bot deployed to Preview November 17, 2020 14:09 View deployment

iamareebjamal reviewed Nov 17, 2020

View reviewed changes

added flash-message when add speaker is clicked

90daf9f

iamareebjamal requested a review from codedsun November 25, 2020 13:22

divyamtayal reviewed Nov 25, 2020

View reviewed changes

Merge branch 'development' into development

0c42856

vercel bot deployed to Preview November 26, 2020 19:40 View deployment

Merge branch 'development' into development

d2aea01

vercel bot deployed to Preview November 27, 2020 15:14 View deployment

changed style of modal flash-message

7a509de

vercel bot deployed to Preview November 27, 2020 16:59 View deployment

sachinchauhan2889 reviewed Nov 27, 2020

View reviewed changes

divyamtayal reviewed Nov 27, 2020

View reviewed changes

designing changes

18f775c

vercel bot deployed to Preview November 27, 2020 19:34 View deployment

sachinchauhan2889 suggested changes Nov 27, 2020

View reviewed changes

Done

164d846

vercel bot deployed to Preview November 27, 2020 20:06 View deployment

sachinchauhan2889 approved these changes Nov 27, 2020

View reviewed changes

iamareebjamal changed the title ~~Login Box: Show info message when page directs to login page~~ feat: Login Box: Show info message when page directs to login page Nov 27, 2020

auto-label bot added the feature label Nov 27, 2020

iamareebjamal approved these changes Nov 27, 2020

View reviewed changes

iamareebjamal merged commit 1433dac into fossasia:development Nov 27, 2020

		@@ -0,0 +1,3 @@
		import FlashObject from 'ember-cli-flash/flash/object';

		FlashObject.reopen({ init() {} });

		@@ -1,4 +1,13 @@
		<div class="ui stackable {{unless this.noSocial 'three' 'one'}} column doubling centered grid">

		@@ -1,3 +1,12 @@
		<div class="flash-messages ui container mb-5">

feat: Login Box: Show info message when page directs to login page #5531

feat: Login Box: Show info message when page directs to login page #5531

Uh oh!

Conversation

Draco9421 commented Nov 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Short description of what this resolves:

Changes proposed in this pull request:

Checklist

Uh oh!

vercel bot commented Nov 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Draco9421 commented Nov 12, 2020

Uh oh!

iamareebjamal commented Nov 12, 2020

Uh oh!

Draco9421 commented Nov 13, 2020

Uh oh!

codecov bot commented Nov 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

iamareebjamal commented Nov 17, 2020

Uh oh!

Draco9421 commented Nov 17, 2020

Uh oh!

iamareebjamal commented Nov 17, 2020

Uh oh!

Draco9421 commented Nov 17, 2020

Uh oh!

iamareebjamal commented Nov 17, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Draco9421 commented Nov 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iamareebjamal commented Nov 17, 2020

Uh oh!

Draco9421 commented Nov 18, 2020

Uh oh!

iamareebjamal commented Nov 18, 2020

Uh oh!

iamareebjamal commented Nov 25, 2020

Uh oh!

divyamtayal left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Change at

Uh oh!

divyamtayal left a comment

Choose a reason for hiding this comment

Uh oh!

iamareebjamal commented Nov 25, 2020

Uh oh!

mariobehling commented Nov 25, 2020

Uh oh!

Draco9421 commented Nov 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iamareebjamal commented Nov 27, 2020

Uh oh!

sachinchauhan2889 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Draco9421 commented Nov 8, 2020 •

edited

Loading

vercel bot commented Nov 8, 2020 •

edited

Loading

codecov bot commented Nov 17, 2020 •

edited

Loading

Draco9421 commented Nov 17, 2020 •

edited

Loading

divyamtayal left a comment •

edited

Loading

Draco9421 commented Nov 25, 2020 •

edited

Loading