Integrate web component to webkit #116

saadnoor · 2021-12-10T18:52:56Z

In this PR:

Integration of web-components into webkit
Avatar Icon as an example implementation

How to test avatar Icon:

go widgets/Drawer/components/DrawerFooter/DrawerFooter.bbj
in line number 58, set #drawerDataModel!.getAvatarUrl() as ""
you will see a cirlcular avatar containing the name of the user.

…rom NPM

…nent-to-webkit-

DominicWrege · 2022-03-18T09:26:50Z

widgets/WebComponents/AvatarIcon/AvatarIcon.bbj

+        sg!.setContext(sg!.getAvailableContext())
+        htmlView! = #Wnd!.addHtmlView(wnd!.getAvailableControlID(),0,0,300,300,"<avatar-initial height="""+height$+""" width="""+width$+""" name="""+name$+"""></avatar-initial>")
+        htmlView! = #configureAvatarView(htmlView!)
+    methodret htmlView!


Syntax Error: missing methodend

DominicWrege · 2022-03-18T09:27:20Z

widgets/WebComponents/AvatarIcon/AvatarIcon.bbj

+        #fireEvent(#ON_AVATAR_INITIAL_CLICK, ev!)
+    methodend
+
+methodend 


Remove this methodend

DominicWrege · 2022-03-18T09:28:02Z

widgets/WebComponents/AvatarIcon/AvatarIcon.bbj

+
+    methodend
+
+    method public BBjHtmlView drawAvatarIcon(BBjWindow wnd!,BBjInt height%, BBjInt width%, BBjString name$)


duplicate code

Hi, it's not duplicate, one function takes integer and other take string (width and height)

We don't need two methods performing the exact same task and executing basically the same thing. The method with the int parameters converts the ints into strings and executes the exact same code as the method with the string parameters, making it duplicate code

ah you're right. Got it. Fixing now. Thanks.

DominicWrege · 2022-03-18T09:28:11Z

widgets/WebComponents/AvatarIcon/AvatarIcon.bbj

+    methodend 
+
+    method public BBjHtmlView drawAvatarIcon(BBjWindow wnd!,BBjString height$, BBjString width$, BBjString name$)
+        htmlView! = wnd!.addHtmlView(wnd!.getAvailableControlID(),0,0,300,300,"<avatar-initial height="""+height$+""" width="""+width$+""" name="""+name$+"""></avatar-initial>")


duplicate code

DominicWrege · 2022-03-18T09:28:32Z

widgets/WebComponents/AvatarIcon/AvatarIcon.bbj

+        sg! =  BBjAPI().openSysGui("X0")
+        #Wnd! =sg!.addWindow(95501,0,0,0,0,"",$01101083$)
+        sg!.setContext(sg!.getAvailableContext())
+        htmlView! = #Wnd!.addHtmlView(wnd!.getAvailableControlID(),0,0,300,300,"<avatar-initial height="""+height$+""" width="""+width$+""" name="""+name$+"""></avatar-initial>")


duplicate code

DominicWrege · 2022-03-18T09:29:11Z

widgets/WebComponents/AvatarIcon/AvatarIcon.bbj

+
+    method public BBjHtmlView drawAvatarIcon(BBjString height$, BBjString width$, BBjString name$)
+        sg! =  BBjAPI().openSysGui("X0")
+        #Wnd! =sg!.addWindow(95501,0,0,0,0,"",$01101083$)


DominicWrege · 2022-03-18T09:29:43Z

widgets/WebComponents/AvatarIcon/AvatarIcon.bbj

+    methodend 
+
+    method public void onClickAvatarInitial(BBjNativeJavaScriptEvent ev!)
+        ClientUtil.consoleLog("catched the webcomponent event from BBj :)")


remove this log

SebastianAdams · 2022-03-18T11:05:20Z

@saadnoor
Can you please include the javascript code in this pull request so that we can also review that part

saadnoor · 2022-03-18T11:37:25Z

@saadnoor
Can you please include the javascript code in this pull request so that we can also review that part that's already in master, in a seperate repo https://github.com/BasisHub/web-components

hyyan · 2022-03-18T11:46:16Z

@saadnoor I've opened several issues for your avatar component in: https://github.com/BasisHub/web-components/issues

SebastianAdams · 2022-03-18T11:44:43Z

demo/Showcase/ShowcasePanels/AvatarInital.bbj

+    method public AvatarInital(BBjWindow wnd!)
+        #super!(wnd!)
+
+        #setTitle("Avatar Initail Demo")


Typo: It should probably say "Avatar Initial Demo"

SebastianAdams · 2022-03-18T11:45:29Z

demo/Showcase/ShowcasePanels/AvatarInital.bbj

+use ::WebKit/widgets/WebComponents/WebComponents.bbj::WebComponents
+use ::WebKit/widgets/WebComponents/AvatarIcon/AvatarIcon.bbj::AvatarIcon
+
+class public AvatarInital extends ShowcaseWidget


Typo in class name: It should probably say "AvatarInitial" or "AvatarInitials"

SebastianAdams · 2022-03-18T11:45:54Z

demo/Showcase/ShowcasePanels/AvatarInital.bbj

@@ -0,0 +1,71 @@
+use ::WebKit/demo/Showcase/ShowcaseWidget/ShowcaseWidget.bbj::ShowcaseWidget


Typo in file name

Code indentation in the whole file is bad

SebastianAdams · 2022-03-18T11:47:02Z

demo/Showcase/ShowcasePanels/AvatarInital.bbj

+        #super!(wnd!)
+
+        #setTitle("Avatar Initail Demo")
+        #setIntro("This demo shows the Avatar initail  stuff")


Typo + We should refrain from writing "stuff" as this seems unprofessional

SebastianAdams · 2022-03-18T11:58:26Z

widgets/common/CircularAvatar/CircularAvatar.bbj

    methodend
+
+    method public CircularAvatar(BBjWindow parent!, BBjString radius!, BBjString imagePath!, BBjString title!)
+        DynamicLoader.addLocalCSS("WebKit/widgets/common/CircularAvatar/CircularAvatar.css")


The DynamicLoader is deprecated, use the ClientUtil instead

SebastianAdams · 2022-03-18T12:00:24Z

widgets/WebComponents/AvatarIcon/AvatarIcon.bbj

+use ::WebKit/widgets/WebComponents/WebComponents.bbj::WebComponents
+use ::WebKit/util/ClientUtil.bbj::ClientUtil
+
+class public AvatarIcon extends WebComponents


This class should follow the BBjWidget pattern and not use some custom methods to use the widget (drawAvatarIcon)

SebastianAdams · 2022-03-18T12:03:43Z

widgets/WebComponents/AvatarIcon/AvatarIcon.bbj

+use ::WebKit/widgets/WebComponents/WebComponents.bbj::WebComponents
+use ::WebKit/util/ClientUtil.bbj::ClientUtil
+
+class public AvatarIcon extends WebComponents


This class is missing some setters to update the avatar values - See BBjWidget pattern

SebastianAdams · 2022-03-18T12:06:29Z

widgets/WebComponents/AvatarIcon/AvatarIcon.bbj

+        methodret htmlView!
+    methodend 
+
+    method public BBjHtmlView drawAvatarIcon(BBjString height$, BBjString width$, BBjString name$)


The class should be a widget meaning that the BBjHtmlView should not be returned. The class might use the BBjHtmlView internally but for external usage it should always be treated as BBjWidget

SebastianAdams · 2022-03-18T12:10:36Z

widgets/WebComponents/AvatarIcon/AvatarIcon.bbj

+    methodend 
+
+    method public BBjHtmlView drawAvatarIcon(BBjWindow wnd!,BBjString height$, BBjString width$, BBjString name$)
+        htmlView! = wnd!.addHtmlView(wnd!.getAvailableControlID(),0,0,300,300,"<avatar-initial height="""+height$+""" width="""+width$+""" name="""+name$+"""></avatar-initial>")


The properties should not be passed using inline css

SebastianAdams · 2022-03-18T12:20:22Z

I was only able to review the code because the showcase didn't work on my machine for unknown reasons

saadnoor · 2022-03-21T15:15:48Z

@saadnoor I've opened several issues for your avatar component in: https://github.com/BasisHub/web-components/issues

@hyyan thanks for opening the issues, will address them one by one

…nent-to-webkit-

saadnoor added 7 commits December 2, 2021 18:13

webcomponents added as submodule

5a74f99

add - successful test implementation of avatar icons.

97e94d5

Fix - get library via npm cdn

819fe75

Removed submodule

fc62869

Fix - submodule trace removal

2b9e0a3

Add - avatar icon full implementation

d793389

Update - CDN Url for always getting the latest version webcomponent f…

828833e

…rom NPM

saadnoor requested a review from StephanWald December 10, 2021 18:53

saadnoor added 10 commits December 27, 2021 23:07

fix - code review feedback

259f142

Merge remote-tracking branch 'origin/master' into Integrate-web-compo…

d260e98

…nent-to-webkit-

fix - code cleanup based of review

285a135

Add - native event capture support

d095629

Add - showcase for avatar Icons

41dbc61

Refactor - Code cleanup on showcase

ee7f5c1

Refactor - Improved code of webcomponents (avatar icon)

2da8fe6

Fix - Updated icons

f7093fa

Merge remote-tracking branch 'origin/master' into Integrate-web-compo…

d7ca88c

…nent-to-webkit-

Fix - Added the dialog showcase again

48ca83e

StephanWald previously approved these changes Feb 17, 2022

View reviewed changes

saadnoor added 2 commits March 17, 2022 15:16

merge conflict resolved

dd08c04

Fix - Updated Id for avatar inital

0fe6673

saadnoor dismissed StephanWald’s stale review via 0fe6673 March 17, 2022 09:25

saadnoor force-pushed the Integrate-web-component-to-webkit- branch from 6d23878 to 0fe6673 Compare March 17, 2022 09:25

saadnoor requested a review from TimonGeisbauer March 17, 2022 09:26

Fix - upper case issue with windows OS

b536857

saadnoor requested a review from DominicWrege March 17, 2022 16:27

StephanWald requested review from SebastianAdams and hyyan and removed request for DominicWrege and TimonGeisbauer March 18, 2022 09:28

DominicWrege suggested changes Mar 18, 2022

View reviewed changes

SebastianAdams requested changes Mar 18, 2022

View reviewed changes

saadnoor added 5 commits March 19, 2022 16:05

Fix - syntax error related method end and removed code duplication

564b64e

Fix - introduced redraw method for avatar initial based on review

a109909

Fix - code cleanup for redraw design in avatar initial

cb68faf

Fix - removed code duplicacy

8406c8e

Fix - avatar initial spelling mistake

0ca6ae4

saadnoor added 3 commits March 22, 2022 19:43

Refactor - Updated circular avatar and removed dynamic loader

d1e22f0

Fix - Updated avatar initial with latest webcomponent changes

a56af55

Merge remote-tracking branch 'origin/master' into Integrate-web-compo…

82db059

…nent-to-webkit-


		methodend

		method public BBjHtmlView drawAvatarIcon(BBjWindow wnd!,BBjInt height%, BBjInt width%, BBjString name$)

		@@ -0,0 +1,71 @@
		use ::WebKit/demo/Showcase/ShowcaseWidget/ShowcaseWidget.bbj::ShowcaseWidget

Integrate web component to webkit #116

Are you sure you want to change the base?

Integrate web component to webkit #116

Uh oh!

Conversation

saadnoor commented Dec 10, 2021 • 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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SebastianAdams commented Mar 18, 2022

Uh oh!

saadnoor commented Mar 18, 2022

Uh oh!

hyyan commented Mar 18, 2022 • 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!

SebastianAdams Mar 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

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!

SebastianAdams commented Mar 18, 2022

Uh oh!

saadnoor commented Mar 21, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

saadnoor commented Dec 10, 2021 •

edited

Loading

hyyan commented Mar 18, 2022 •

edited

Loading

SebastianAdams Mar 18, 2022 •

edited

Loading