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

[Bug] Fix UI Element text Breaking into two lines when long username #46

Closed
Nabhag8848 opened this issue Jan 28, 2024 · 25 comments · Fixed by #54
Closed

[Bug] Fix UI Element text Breaking into two lines when long username #46

Nabhag8848 opened this issue Jan 28, 2024 · 25 comments · Fixed by #54
Assignees
Labels
bug Something isn't working

Comments

@Nabhag8848
Copy link
Collaborator

What happened?

  • onInstallation message and when running /notion connect text in notification is breaking into two lines. we need to make an notification such that it appears in single line even though username is large.

  • Incase of onInstallation message we need to attach the layout to top the the Hey username welcome to notion app message below the layout.

Steps to reproduce

  • /notion connect with large username.
  • install app having large username

Relevant ScreenShots

Screen.Recording.2024-01-29.at.00.25.37.mov
pic1

Version

v6.5

What browsers are you seeing the problem on?

Chrome

@Nabhag8848 Nabhag8848 added the bug Something isn't working label Jan 28, 2024
@brf153
Copy link
Contributor

brf153 commented Jan 28, 2024

Incase of onInstallation message we need to attach the layout to top the the Hey username welcome to notion app message below the layout.

@Nabhag8848 I did not get this

@Nabhag8848
Copy link
Collaborator Author

Incase of onInstallation message we need to attach the layout to top the the Hey username welcome to notion app message below the layout.

@Nabhag8848 I did not get this

I mean Hey username welcome to notion app that text needs below layout so layout info will stick to top. meaning Hey username welcome to notion app should be first line incase of setting up the notion app..

@Spiral-Memory
Copy link
Contributor

@Nabhag8848 , I feel the issue lies here LayoutBlockType.SECTION only occupies a certain width. Both of the issue you mentioned, in that the text uses the function createSectionBlock and hence causing the issue. I am new to Rocket chat UI Kit, so i am not sure, which will be better to use in this case ?

image

Using getCreator of IModify, full line text can be done, then using createContextBlock that is custom function defined, it can be done.

@Nabhag8848
Copy link
Collaborator Author

@Nabhag8848 , I feel the issue lies here LayoutBlockType.SECTION only occupies a certain width. Both of the issue you mentioned, in that the text uses the function createSectionBlock and hence causing the issue. I am new to Rocket chat UI Kit, so i am not sure, which will be better to use in this case ?

image

Using getCreator of IModify, full line text can be done, then using createContextBlock that is custom function defined, it can be done.

Hey, I believe @brf153 is working on this issue. + please please please only work on one issue at a time.

@Spiral-Memory
Copy link
Contributor

@Nabhag8848 , Certainly, sir. I apologize if it caused any inconvenience. Well, which issue should I work on, sir? Apart from preserving the message one, as that is a significant issue and requires a lot of discussion to fully understand the implementation. Currently, I am looking for a good first issue to understand the codebase and how RC apps work.

@brf153
Copy link
Contributor

brf153 commented Jan 29, 2024

@Nabhag8848 I believe there is an issue. For testing purposes, I created a function named sendNotificationWithBlock.
Screenshot from 2024-01-29 11-54-53

Initially, you can observe that I have commented out some code, and I'm not using any blocks at this point. As shown in the picture below, the text is fully visible and does not break.
Screenshot from 2024-01-29 11-57-12

When I remove the comment (i.e., when I add the block), I only get the block and not the message.
Screenshot from 2024-01-29 12-11-19

The section block, which we were using earlier, does not allow us to increase the width, causing the text to break.

We have two options: either replace the text with something else like Hello!👋 Connect your Notion Workspace without including the name, or consider making an issue in the apps-engine and try to address the problem there.

@Nabhag8848
Copy link
Collaborator Author

@Nabhag8848 I believe there is an issue. For testing purposes, I created a function named sendNotificationWithBlock. Screenshot from 2024-01-29 11-54-53

Initially, you can observe that I have commented out some code, and I'm not using any blocks at this point. As shown in the picture below, the text is fully visible and does not break. Screenshot from 2024-01-29 11-57-12

When I remove the comment (i.e., when I add the block), I only get the block and not the message. Screenshot from 2024-01-29 12-11-19

The section block, which we were using earlier, does not allow us to increase the width, causing the text to break.

We have two options: either replace the text with something else like Hello!👋 Connect your Notion Workspace without including the name, or consider making an issue in the apps-engine and try to address the problem there.

yep. new uikit sets either text or block at a time.
Can we have button as an attachment does that solve our problem or we would face the same issue ? if that doesn't we can change text to Hey ${username} !.

@brf153
Copy link
Contributor

brf153 commented Jan 29, 2024

Oki I will check it

@brf153
Copy link
Contributor

brf153 commented Jan 30, 2024

@Nabhag8848 I have implemented the functionality you stated above. I am sending the action block as an attachment. Although the UI might not be perfect, it is working fine.
Screenshot from 2024-01-30 12-09-00

@brf153
Copy link
Contributor

brf153 commented Jan 30, 2024

Shall I raise a pr for these changes?

@Nabhag8848
Copy link
Collaborator Author

Nabhag8848 commented Jan 30, 2024

Shall I raise a pr for these changes?

Can we make a button variant to primary so we can have a blue background.+ change text to Connect to Workspace to maintain consistency across app. and about OnInstallation feature we have come up with solution and are we able to achieve as we intended ?

@brf153
Copy link
Contributor

brf153 commented Jan 30, 2024

@Nabhag8848 I have made the changes for onInstall. It looks like
Screenshot from 2024-01-30 14-39-49

@brf153
Copy link
Contributor

brf153 commented Jan 30, 2024

Can we add custom css to our code for the ui kit? I am not being able to add the style: "primary" to the attachment's block. But, when I make changes in the devtools of chrome, I am being able to make the required changes.
Screenshot from 2024-01-30 14-49-10

@brf153
Copy link
Contributor

brf153 commented Jan 30, 2024

Without custom css our button and message will look like
Screenshot from 2024-01-30 14-56-23

@Nabhag8848
Copy link
Collaborator Author

Can we add custom css to our code for the ui kit? I am not being able to add the style: "primary" to the attachment's block. But, when I make changes in the devtools of chrome, I am being able to make the required changes. Screenshot from 2024-01-30 14-49-10

we can't as far as i know, hacks can go again RC design principles.

@Nabhag8848
Copy link
Collaborator Author

Without custom css our button and message will look like Screenshot from 2024-01-30 14-56-23

Are we using PRIMARY button anywhere else except the submit button of modal and Comment Button ? if not, this won't break consistency across app, we are good to go with this.

@Nabhag8848
Copy link
Collaborator Author

Nabhag8848 commented Jan 30, 2024

@Nabhag8848 I have made the changes for onInstall. It looks like Screenshot from 2024-01-30 14-39-49

Can we have that greeting on the second message ? also as we see, having two notification on install, can we make it one somehow and match the UI ? Can we have that layout as attachment and look as if it is attached below the notification with setup information like connect to workspace button, if thats possible lemme know.

@brf153
Copy link
Contributor

brf153 commented Jan 30, 2024

Without custom css our button and message will look like Screenshot from 2024-01-30 14-56-23

Are we using PRIMARY button anywhere else except the submit button of modal and Comment Button ? if not, this won't break consistency across app, we are good to go with this.

Screenshot from 2024-01-30 19-26-43

Shall I make these the same as I have done for the /notion connect command?

@brf153
Copy link
Contributor

brf153 commented Jan 30, 2024

Screenshot from 2024-01-30 19-29-02
By submit button of modal, do you mean these buttons?

@brf153
Copy link
Contributor

brf153 commented Jan 30, 2024

@Nabhag8848 I have made the changes for onInstall. It looks like Screenshot from 2024-01-30 14-39-49

Can we have that greeting on the second message ? also as we see, having two notification on install, can we make it one somehow and match the UI ? Can we have that layout as attachment and look as if it is attached below the notification with setup information like connect to workspace button, if thats possible lemme know.

I can make something like
Screenshot from 2024-01-30 20-17-37
I think the layout you are talking about is the preview block which is having the notion's image, the header, the footer, and the text element. I have checked but I did not get anyway to send blocks as the attachment. The only block which we can send is the action block which is already defined in the IMessageAttachment

@Nabhag8848
Copy link
Collaborator Author

@Nabhag8848 I have made the changes for onInstall. It looks like Screenshot from 2024-01-30 14-39-49

Can we have that greeting on the second message ? also as we see, having two notification on install, can we make it one somehow and match the UI ? Can we have that layout as attachment and look as if it is attached below the notification with setup information like connect to workspace button, if thats possible lemme know.

I can make something like Screenshot from 2024-01-30 20-17-37 I think the layout you are talking about is the preview block which is having the notion's image, the header, the footer, and the text element. I have checked but I did not get anyway to send blocks as the attachment. The only block which we can send is the action block which is already defined in the IMessageAttachment

@brf153 This Looks Good !

@Nabhag8848
Copy link
Collaborator Author

Nabhag8848 commented Feb 4, 2024

Without custom css our button and message will look like Screenshot from 2024-01-30 14-56-23

Are we using PRIMARY button anywhere else except the submit button of modal and Comment Button ? if not, this won't break consistency across app, we are good to go with this.

Screenshot from 2024-01-30 19-26-43

Shall I make these the same as I have done for the /notion connect command?

@brf153 having button as attachment would break this buttons variation and will force us to change that to grey (not primary) which we can avoid, as mentioned earlier.

@Nabhag8848
Copy link
Collaborator Author

@brf153 Lets just have Connect your Notion Workspace
Final

@brf153
Copy link
Contributor

brf153 commented Feb 5, 2024

@Nabhag8848 what I understood is
i) I will change the onInstall text to
Screenshot from 2024-01-30 20-17-37
ii) I will simply change the /notion connect text to Connect you Notion Workspace.

Do I need to make any further changes?

@Nabhag8848
Copy link
Collaborator Author

@Nabhag8848 what I understood is
i) I will change the onInstall text to
Screenshot from 2024-01-30 20-17-37
ii) I will simply change the /notion connect text to Connect you Notion Workspace.

Do I need to make any further changes?

looks good. Good to go for making changes to PR🚀

@Nabhag8848 Nabhag8848 added this to Pending Review in Notion App - Community Dashboard Feb 29, 2024
@Nabhag8848 Nabhag8848 moved this from Pending Review to To Do in Notion App - Community Dashboard Feb 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants