[ Bug ] Removed emojis from the app #40

brf153 · 2024-01-19T13:48:13Z

Issue(s)

closes #37

Acceptance Criteria fulfillment

Reported the emojis in the issue section
Removed the emojis from the code

Proposed changes (including videos or screenshots)

WhatsApp.Video.2024-01-19.at.19.14.29.mp4

Further comments

@Nabhag8848 I could not get the code for the "more" dropdown menu.

Signed-off-by: brf153 <[email protected]>

Nabhag8848 · 2024-01-19T13:58:32Z

Find it here
https://github.com/RocketChat/Apps.Notion/blob/main/i18n%2Fen.json

@brf153
learn about how apps uses i18n translation.

Can you open up the Modal
/notion create db
and see the dropdown if there are no emojis fix that cause we need there to differentiate between page and db.

brf153 · 2024-01-19T14:06:29Z

@Nabhag8848 I have removed the emojis from the i18n files. Still the emoji is there in the menu dropdown

Nabhag8848 · 2024-01-19T14:07:30Z

@Nabhag8848 I have removed the emojis from the i18n files. Still the emoji is there in the menu dropdown

Refresh Rocketchat it will not show up anymore.

brf153 · 2024-01-19T14:07:35Z

brf153 · 2024-01-19T14:08:44Z

@Nabhag8848 I have removed the emojis from the i18n files. Still the emoji is there in the menu dropdown

Refresh Rocketchat it will not show up anymore.

Yeah my bad it is not showing after refreshing the page.

brf153 · 2024-01-19T14:09:01Z

I think I have removed all the emojis

Nabhag8848 · 2024-01-19T14:09:26Z

Find it here
https://github.com/RocketChat/Apps.Notion/blob/main/i18n%2Fen.json

@brf153
learn about how apps uses i18n translation.

Can you open up the Modal
/notion create db
and see the dropdown if there are no emojis fix that cause we need there to differentiate between page and db.

edited this check: i meant
/notion create

brf153 · 2024-01-19T14:10:14Z

brf153 · 2024-01-19T14:11:58Z

see the dropdown if there are no emojis fix that cause we need there to differentiate between page and db. I did not get this line.

Nabhag8848 · 2024-01-19T14:17:48Z

see the dropdown if there are no emojis fix that cause we need there to differentiate between page and db. I did not get this line.

Feature 5 under Demo:

https://github.com/Nabhag8848/Google-Summer-of-Code

See those two videos on that same modal when we select Page, Modal doesn't change to include other components as when we select page it is meant to create subpage within notion but in case when we select db ( option with pile of books 📚 it will update the modal to include other components as it this time it is meant to create record or row in a notion table.

if there are no emojis how we can differentiate if its a table or page? thats the issue.

brf153 · 2024-01-19T14:21:57Z

Oki so, should I add the page emoji in front of pages and books emoji in front of database?

Nabhag8848 · 2024-01-19T14:23:37Z

Oki so, should I add the page emoji in front of pages and books emoji in front of database?

#37 (comment)

Signed-off-by: brf153 <[email protected]>

brf153 · 2024-01-19T14:43:16Z

@Nabhag8848 I have added emoji:boolean in the function which will check whether the emoji needs to be rendered or not

brf153 · 2024-01-21T07:17:47Z

@Nabhag8848 is something left to be done?

Nabhag8848 · 2024-01-21T07:19:37Z

@Nabhag8848 is something left to be done?

Will review and test on my end and will let you know.

brf153 · 2024-01-21T07:20:54Z

Oki

Nabhag8848 · 2024-01-23T05:52:40Z

@brf153
Some breaking changes:

When Selecting Database in /notion create We are showing Emojis in layout: remove that.

Missed out 🚫 under /enum/OAuth2.ts and /src/handlers/ExecuteViewSubmitHandler.ts, just search "🚫" under IDE, Note: we don't need to remove it from webhook.ts folder as it is for AuthTemplate.

brf153 · 2024-01-23T05:59:49Z

@Nabhag8848 I will fix this right now

Nabhag8848

Made Some Changes Comments.

Nabhag8848 · 2024-01-23T06:00:18Z

README.md

oops, i believe you removed from documentation which is not needed.

Should I add the emojis in the documentation?

yep, revert changes of docs.

Nabhag8848 · 2024-01-23T06:00:53Z

enum/OAuth2.ts

-    NOT_CONNECTED_MESSAGE = `👋 You are not connected to **Workspace**!`,
-    NOT_CONNECTED_MESSAGE_WITH_INFO = `👋 Connect to workspace to access \`pages\` & \`database\``,
+    NOT_CONNECTED_MESSAGE = `You are not connected to **Workspace**!`,
+    NOT_CONNECTED_MESSAGE_WITH_INFO = `Connect to workspace to access \`pages\` & \`database\``,
    CONNECT_TO_WORKSPACE = "Connect to Workspace",
    CREDENTIALS_MISSING_USER = `🚫 Something Went Wrong, Please Contact the Admin!`,


Remove 🚫

Nabhag8848 · 2024-01-23T06:02:14Z

src/endpoints/webhook.ts

@@ -113,7 +113,7 @@ export class WebHookEndpoint extends ApiEndpoint {
        const successTemplate = getAuthPageTemplate(
            "Connected to Workspace",
            OAuth2Content.success,
-            `👋 Connected to ${response.workspace_name}❗`,


Add 👋 as it is AuthPageTemplate as we discussed earlier.

Nabhag8848 · 2024-01-23T06:03:01Z

src/handlers/ExecuteViewSubmitHandler.ts

@@ -387,7 +387,7 @@ export class ExecuteViewSubmitHandler {
            message = `🚫 Something went wrong while creating page in **${workspace_name}**.`;


Remove 🚫. there are several others in same file remove all of them.

I will try to remove it from wherever it is required. I don't know why but my vs code search does not show any data when I search for "🚫"

Nabhag8848 · 2024-01-23T06:04:41Z

src/lib/NotionSDK.ts

        }

        return null;
    }

-    private returnPage(name: string, page_id: string): IPage {
+    private returnPage(name: string, page_id: string, emoji: boolean): IPage {


can we make emoji parameter default value as false to avoid passing when we don't need emojis.

Nabhag8848 · 2024-01-23T06:05:24Z

src/lib/NotionSDK.ts

@@ -540,7 +541,7 @@ export class NotionSDK implements INotionSDK {
        }
    }

-    private async getDatabaseObjectFromResults(item): Promise<IDatabase> {
+    private async getDatabaseObjectFromResults(item, emoji:boolean): Promise<IDatabase> {


Same here: Emoji default value as false and make necessary changes whereever you are calling this function.

… and IDatabase in createPage and createRecord Signed-off-by: brf153 <[email protected]>

Signed-off-by: brf153 <[email protected]>

brf153 · 2024-01-23T06:51:40Z

@Nabhag8848 I have made the changes

Signed-off-by: brf153 <[email protected]>

brf153 · 2024-01-23T08:04:16Z

@Nabhag8848

WhatsApp.Video.2024-01-23.at.13.32.46.mp4

brf153 · 2024-01-23T08:43:03Z

in this pic, I am also removing the emoji

brf153 · 2024-01-23T09:06:46Z

I have made the changes

Signed-off-by: brf153 <[email protected]>

Nabhag8848 · 2024-01-25T07:08:02Z

src/handlers/ExecuteViewSubmitHandler.ts

+        const newDatabase = {
+            ...database,
+            info: {
+                name: database.info.name.replace("📚 ",""),
+                link: database.info.link,
+            },
+        }


I believe, you overriding this due to when sending message to channel it was showing 📚. can't directly we can have const databasename = info.name.replace("📚",""); in line 504. mention below. so we don't need to have this newDatabase object

Suggested change

const newDatabase = {

...database,

info: {

name: database.info.name.replace("📚 ",""),

link: database.info.link,

},

}

Yeah, that is better. I am making the changes.

Nabhag8848 · 2024-01-25T07:09:04Z

src/handlers/ExecuteViewSubmitHandler.ts

            await sendNotification(this.read, this.modify, user, room, {
                message,
            });
        } else {
-            const { info } = database;
+            const { info } = newDatabase;
            const databasename = info.name;


Here, as i mentioned above.

Suggested change

const databasename = info.name;

const databasename = info.name.replace("📚 ","");

Nabhag8848 · 2024-01-25T07:14:58Z

src/helper/getSelectDatabaseLayout.ts

@@ -15,6 +15,7 @@ export function getSelectDatabaseLayout(

    const database_name = properties.name;
    const database_url = properties.link;
+    const databaseNameWithoutEmoji = database_name.replace("📚 ", "")


We can directly replace emoji in database_name variable to avoid creating new variable.

line 16: const database_name = properties.name.replace("📚 ", "");

Suggested change

const databaseNameWithoutEmoji = database_name.replace("📚 ", "")

Nabhag8848 · 2024-01-25T07:21:28Z

src/lib/NotionSDK.ts

@@ -989,7 +991,8 @@ export class NotionSDK implements INotionSDK {

            const pageInfo = response.data;
            const page = (await this.getPageObjectFromResults(
-                pageInfo
+                pageInfo,
+                false


Previously we made a emoji param default value false so that we don't need to pass it when its false, remove from every calls where it is false. there are at several other places.

Suggested change

false

Oki making the changes

Nabhag8848 · 2024-01-25T07:23:28Z

src/lib/NotionSDK.ts

@@ -132,7 +132,7 @@ export class NotionSDK implements INotionSDK {
        }
    }

-    private async getPageObjectFromResults(item): Promise<IPage | null> {
+    private async getPageObjectFromResults(item, emoji:boolean=false): Promise<IPage | null> {


As you are making changes in this file with suggestions, make a indentation with other methods too which has default value of emoji.

Suggested change

private async getPageObjectFromResults(item, emoji:boolean=false): Promise<IPage | null> {

private async getPageObjectFromResults(item, emoji:boolean = false): Promise<IPage | null> {

Nabhag8848 · 2024-01-25T07:25:47Z

src/lib/NotionSDK.ts

            const { name, parent } = page;
            const { title } = prop;
+            const nameWithoutEmoji = name.replace("📄", "");


we can avoid long variable name here, make neccessary changes which would affect this below.

Suggested change

const { name, parent } = page;

const { title } = prop;

const nameWithoutEmoji = name.replace("📄", "");

const { parent } = page;

const { title } = prop;

const name = page.name.replace("📄", "");

Nabhag8848

@brf153 Good work! Few More Changes, Hope those comments help you in future PRs.

Signed-off-by: brf153 <[email protected]>

brf153 · 2024-01-25T08:01:24Z

@Nabhag8848 Thank you for your detailed feedback and comments! I appreciate the guidance, and I'll make sure to incorporate these changes in my future PRs. Also, I have made the required changes.

Nabhag8848

Hey @brf153 remove console.log, and follow along other comments.

Nabhag8848 · 2024-01-27T05:45:35Z

src/handlers/ExecuteViewSubmitHandler.ts

            await sendNotification(this.read, this.modify, user, room, {
                message,
            });
        } else {
            const { info } = database;
-            const databasename = info.name;
+            const databasename = info.name.replace("📚 ", "");
+            console.log("databaseName ", databasename)


remove console.log

Suggested change

console.log("databaseName ", databasename)

Nabhag8848 · 2024-01-27T05:46:07Z

src/lib/NotionSDK.ts

        }

        return null;
    }

-    private returnPage(name: string, page_id: string): IPage {
+    private returnPage(name: string, page_id: string, emoji: boolean=false): IPage {


Suggested change

private returnPage(name: string, page_id: string, emoji: boolean=false): IPage {

private returnPage(name: string, page_id: string, emoji: boolean = false): IPage {

Nabhag8848 · 2024-01-27T05:46:45Z

src/lib/NotionSDK.ts

@@ -597,7 +599,7 @@ export class NotionSDK implements INotionSDK {

            let result: INotionPage = {
                link: response?.data?.url,
-                name,
+                name: name,


Suggested change

name: name,

name,

Signed-off-by: brf153 <[email protected]>

brf153 · 2024-01-27T07:31:00Z

@Nabhag8848 made the changes

Nabhag8848

LGTM @brf153 !! 🚀

Spiral-Memory · 2024-02-02T08:10:11Z

Hey @brf153 , I am not sure what is causing it, but after this merge has been completed, I am noticing a thin line over the dropdown that is flickering while typing something. I'm not sure what is causing this issue. I saw your code, and I wasn't able to figure out what is causing the issue, so I am just letting you know that this is happening. If you can figure out what might be causing it, you can fix it. I tried reverting your changes and then that thin line disappeared, so i feel some changes might be causing this.

Note: I am experiencing the issue only in Microsoft Edge browser in Ubuntu 22.0, I tried using other browsers, and it was working fine. However after reverting the commit, in Edge also, there is no such line.

brf153 · 2024-02-02T08:57:55Z

@Spiral-Memory I don't have it bro

Spiral-Memory · 2024-02-02T09:05:59Z

Can you check it in dark mode in the Microsoft edge browser ?

feat:removed emojis from the app

9c67f0e

Signed-off-by: brf153 <[email protected]>

feat:added conditional rendering of emojis for page and database

7f75205

Signed-off-by: brf153 <[email protected]>

Nabhag8848 requested changes Jan 23, 2024

View reviewed changes

brf153 added 3 commits January 23, 2024 12:16

fix:removed the 🚫 emoji along with page and database emoji from IPage…

da8f382

… and IDatabase in createPage and createRecord Signed-off-by: brf153 <[email protected]>

fix:set default value of emoji to false

af90315

Signed-off-by: brf153 <[email protected]>

code cleaned

2a4ca7a

Signed-off-by: brf153 <[email protected]>

fix:added emojis to the readme

eed2116

Signed-off-by: brf153 <[email protected]>

brf153 requested a review from Nabhag8848 January 23, 2024 08:35

feat:removed emojis from getSelectDatabaseLayout

56b7798

Signed-off-by: brf153 <[email protected]>

Nabhag8848 reviewed Jan 25, 2024

View reviewed changes

Nabhag8848 requested changes Jan 25, 2024

View reviewed changes

enhance: added required changes

b7b34b7

Signed-off-by: brf153 <[email protected]>

brf153 requested a review from Nabhag8848 January 25, 2024 08:01

Nabhag8848 requested changes Jan 27, 2024

View reviewed changes

fix:made the required changes

77d9103

Signed-off-by: brf153 <[email protected]>

brf153 requested a review from Nabhag8848 January 27, 2024 07:31

Nabhag8848 approved these changes Jan 28, 2024

View reviewed changes

Nabhag8848 changed the title ~~[ Feat ]: Removed emojis from the app~~ [ Bug ] Removed emojis from the app Jan 31, 2024

Nabhag8848 merged commit 3c32921 into RocketChat:main Jan 31, 2024
1 check passed

		@@ -387,7 +387,7 @@ export class ExecuteViewSubmitHandler {
		message = `🚫 Something went wrong while creating page in ${workspace_name}.`;

	const databasename = info.name;
	const databasename = info.name.replace("📚 ","");

	private async getPageObjectFromResults(item, emoji:boolean=false): Promise<IPage \| null> {
	private async getPageObjectFromResults(item, emoji:boolean = false): Promise<IPage \| null> {

	private returnPage(name: string, page_id: string, emoji: boolean=false): IPage {
	private returnPage(name: string, page_id: string, emoji: boolean = false): IPage {

[ Bug ] Removed emojis from the app #40

[ Bug ] Removed emojis from the app #40

Conversation

brf153 commented Jan 19, 2024 • edited Loading

Issue(s)

Acceptance Criteria fulfillment

Proposed changes (including videos or screenshots)

Further comments

Nabhag8848 commented Jan 19, 2024 • edited Loading

brf153 commented Jan 19, 2024

Nabhag8848 commented Jan 19, 2024

brf153 commented Jan 19, 2024

brf153 commented Jan 19, 2024

brf153 commented Jan 19, 2024

Nabhag8848 commented Jan 19, 2024

brf153 commented Jan 19, 2024

brf153 commented Jan 19, 2024

Nabhag8848 commented Jan 19, 2024 • edited Loading

brf153 commented Jan 19, 2024

Nabhag8848 commented Jan 19, 2024

brf153 commented Jan 19, 2024

brf153 commented Jan 21, 2024

Nabhag8848 commented Jan 21, 2024

brf153 commented Jan 21, 2024

Nabhag8848 commented Jan 23, 2024 • edited Loading

brf153 commented Jan 23, 2024

Nabhag8848 left a comment

Choose a reason for hiding this comment

Nabhag8848 Jan 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nabhag8848 Jan 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brf153 commented Jan 23, 2024

brf153 commented Jan 23, 2024

brf153 commented Jan 23, 2024

brf153 commented Jan 23, 2024

Nabhag8848 Jan 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nabhag8848 Jan 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nabhag8848 Jan 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nabhag8848 left a comment

Choose a reason for hiding this comment

brf153 commented Jan 25, 2024

Nabhag8848 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brf153 commented Jan 27, 2024

Nabhag8848 left a comment

Choose a reason for hiding this comment

Spiral-Memory commented Feb 2, 2024 • edited Loading

brf153 commented Feb 2, 2024

Spiral-Memory commented Feb 2, 2024

brf153 commented Jan 19, 2024 •

edited

Loading

Nabhag8848 commented Jan 19, 2024 •

edited

Loading

Nabhag8848 commented Jan 19, 2024 •

edited

Loading

Nabhag8848 commented Jan 23, 2024 •

edited

Loading

Nabhag8848 Jan 23, 2024 •

edited

Loading

Nabhag8848 Jan 23, 2024 •

edited

Loading

Nabhag8848 Jan 25, 2024 •

edited

Loading

Nabhag8848 Jan 25, 2024 •

edited

Loading

Nabhag8848 Jan 25, 2024 •

edited

Loading

Spiral-Memory commented Feb 2, 2024 •

edited

Loading