Closes: #39 Add a way to specify the position of child components. #70

SuelenKarbivnychyy · 2023-07-27T19:47:06Z

Added special tag which specify position of inner content of it's parent component

In component class I defined new list to store inner content of special tag . I also defined a variable called is_costum "meaning that is our tag" to be True. In parser.py I check if the parent node is our special tag '', and if it's true, the current component is appended to the inner content list instead of the children list. Then in the dom_methods.py I render the component properties of inner_content list. For every child in children list, I check if the child tag is our , and if it's true, I added the component inner content to its own properties.

BREAKING CHANGE: this closes issue #39

SuelenKarbivnychyy · 2023-07-27T19:47:27Z

@sansyrox Please review.

test-application/parent.fyre

.gitignore

test-application/store.fyre

starfyre/component.py

sansyrox

Hey @SuelenKarbivnychyy 👋

Thank you for the PR. I have left some reviews on the PR.

Also, I don't think we require the test/ folder in the repo. Can you please remove it?

Do let me know if you require any more information from me 😄

SuelenKarbivnychyy · 2023-08-01T14:58:43Z

Do let me know if you require any more information from me 😄

Hey, Yes. I'll remove it. I'm still getting use to Commitzen 😄
Also, Would you be ok of using as our special tag, or would you suggest other name?

starfyre/dom_methods.py

sansyrox

Hey @SuelenKarbivnychyy 👋

Thank you for answering the questions. I have some suggestions for the design. Do let me know if you have any questions 😄

starfyre/dom_methods.py

starfyre/parser.py

starfyre/component.py

starfyre/dom_methods.py

sansyrox

Hey @SuelenKarbivnychyy 👋

Great work! I have some suggestions 😄

Do let me know if you have questions from me :D

…: Added special tag <slot> which specify position of inner content of it's parent component In component class I defined new list to store inner content of special tag <slot>. I also defined a variable called is_costum "meaning that is our <slot> tag" to be True. In parser.py I check if the parent node is our special tag '<slot>', and if it's true, the current component is appended to the inner content list instead of the children list. Then in the dom_methods.py I render the component properties of inner_content list. For every child in children list, I check if the child tag is our <slot>, and if it's true, I added the <slot> component inner content to its own properties. BREAKING CHANGE: this closes issue 39

…hild Implemented code with a variable is_slot_used = False. Then if childrenElement.tag is the <slot>, I set is_slot_used equals True. Then if the <slot> tag is not used in the component definition but we have content in inner_html, that means the user forgot "or is not aware" that he should specify where the children content should be added. Based on that I raise an error to let user knows that he is missing the <slot> tag.

…updated the code to instead of raising a error when <slot> is not found, update the html, js and css anyway

…ation that will be deleted before merging the pr

…cture to follow design best practice implemented the code to make usage of children list instead of using a separate list to handle the custom component content we want to add to the slot

sansyrox · 2023-08-12T19:55:44Z

@SuelenKarbivnychyy 👋

Updated. Let me know if you still have any questions.

starfyre/parser.py

1. Put nodes into current_children

starfyre/parser.py

SuelenKarbivnychyy · 2023-08-28T21:26:15Z

Hey @SuelenKarbivnychyy 👋

Where are we testing for slots in the code base?

@sansyrox
There is <slot> tag on <parent.fyre> file on line 20

deleted extra <div> tag that didnt have closing tag for it.

sansyrox · 2023-08-29T18:50:22Z

starfyre/parser.py

+                new_children = []
+                is_slot_used = False
+                for child_component in endtag_node.children:                        
+                    if child_component.is_slot_component:
                        new_children.extend(self.current_children)


@SuelenKarbivnychyy , we should investigate this line. This is the line that determines the order of rendering of slot elements.

I'm on it :)

sansyrox

Hey @SuelenKarbivnychyy 👋

I have added a testing application(called test-quote) based on the current logic but it is not working as expected.

SuelenKarbivnychyy · 2023-08-30T21:14:40Z

Hey @SuelenKarbivnychyy 👋

I have added a testing application(called test-quote) based on the current logic but it is not working as expected.

I'm looking into that now :)

…into sk-39 getting test code

Fixed the text node order of slot components and implemented a approach to have the correct behavior when custom compoment is the root node in __innit__.fyre BREAKING CHANGE: sparckles#39

sansyrox · 2023-09-01T11:03:33Z

starfyre/parser.py

        self.current_depth -= 1

        if endtag_node.tag != "style" and endtag_node.tag != "script":
+
+            if endtag_node.original_name != endtag_node.tag: #We need to check if the endtag_node is a custom tag


Niceee. There is one final suggestion and everything should be hopefully good to go. I will complete my review by tonight. 😄

sansyrox

Hey @SuelenKarbivnychyy

This has stopped working in the current code however it is working fine in the main branch

SuelenKarbivnychyy · 2023-09-01T18:36:53Z

Hey @SuelenKarbivnychyy
This has stopped working in the current code however it is working fine in the main branch

@sansyrox I'm running the main branch on my pc and the use_parent_signal() as well as get_parent_signal() are not working "And I don't remember seeing it working before". Only the ssr_request() is working as of now.

sansyrox · 2023-09-02T01:35:00Z

test-application/parent.fyre

      <ul>
-        <li>a</li>
+        <li>a</li>        


extra spaces

sansyrox · 2023-09-02T01:35:04Z

test-application/parent.fyre

        <li>b</li>
-        <li>c</li>
+        <li>c</li>        


sansyrox · 2023-09-02T01:35:46Z

starfyre/parser.py

-            data,
-        )
+
+        #We need to check if the wrapped component should go to the custom list aka self.current_children or to parent_node children list


can you please explain a bit more here?

can you please explain a bit more here?

@sansyrox
If the text node is children of a custom tag aka <parent></parent>,
that text node should go to self.current_children list that will be the replacement for the slot.
If we don't check that, the text node will be added straight to the parent_node.children and consequently placed in the wrong order on the final HTML.

@SuelenKarbivnychyy 👋

Can you please articulate this explanation as a code comment? and include it there?

starfyre/parser.py

sansyrox · 2023-09-02T01:48:53Z

starfyre/parser.py

@@ -18,17 +18,41 @@ def extract_functions(obj):


 class ComponentParser(HTMLParser):
-    # this is the grammar for the parser
-    # we need cover all the grammar rules
+    """


@SuelenKarbivnychyy , I have added some explanation. To summarise what we have done so far.

included comments when processing data on parser

sansyrox · 2023-09-05T21:21:32Z

@SuelenKarbivnychyy , can you please add the TODO comment and raise the issue we talked about?

added a TODO to investigate why nested slot component is not rendered correct

SuelenKarbivnychyy · 2023-09-05T21:59:04Z

@SuelenKarbivnychyy , can you please add the TODO comment and raise the issue we talked about?

Yes, I just did it. :)

sansyrox · 2023-09-07T01:41:11Z

starfyre/parser.py

+                # if the tag is not found in the generic tags but found in custom components
+                # we need to replace the tag with the actual component
+                # and add the children to the component
+                # @suelen can you come up with a better explanation for this?                


@SuelenKarbivnychyy , this is still left

@SuelenKarbivnychyy , this is still left

@sansyrox
We already have this active discussion.

I can not explain your reasons/or why is better to have the check for closing custom component tag after processing slots component.

From my understanding, slot tag is meant to be specified on the custom component definition "parent.fyre definition" so we must check if the closing tag is a custom tag before processing slot.

To conclude, my solution is to process slot elements when we're handling closing tag for custom component. That is why check for custom component should be before slot processing. I'm not sure how can I explain proposed logic when this check happens after slot processing.

starfyre/parser.py

sansyrox

@SuelenKarbivnychyy , one last thing to edit and then we can merge this PR 😄

sansyrox reviewed Jul 27, 2023

View reviewed changes

test-application/parent.fyre Outdated Show resolved Hide resolved

SuelenKarbivnychyy force-pushed the sk-39 branch from df6f8d2 to df46194 Compare July 28, 2023 17:35

sansyrox reviewed Jul 29, 2023

View reviewed changes

.gitignore Outdated Show resolved Hide resolved

sansyrox reviewed Jul 29, 2023

View reviewed changes

test-application/store.fyre Outdated Show resolved Hide resolved

sansyrox reviewed Jul 29, 2023

View reviewed changes

starfyre/component.py Outdated Show resolved Hide resolved

sansyrox reviewed Jul 29, 2023

View reviewed changes

SuelenKarbivnychyy force-pushed the sk-39 branch from aa7aa7c to 6d3fb6c Compare August 1, 2023 21:57

sansyrox reviewed Aug 4, 2023

View reviewed changes

starfyre/dom_methods.py Outdated Show resolved Hide resolved

sansyrox reviewed Aug 4, 2023

View reviewed changes

starfyre/dom_methods.py Outdated Show resolved Hide resolved

starfyre/dom_methods.py Outdated Show resolved Hide resolved

starfyre/parser.py Outdated Show resolved Hide resolved

sansyrox reviewed Aug 8, 2023

View reviewed changes

starfyre/component.py Outdated Show resolved Hide resolved

sansyrox reviewed Aug 8, 2023

View reviewed changes

starfyre/component.py Outdated Show resolved Hide resolved

sansyrox reviewed Aug 8, 2023

View reviewed changes

starfyre/dom_methods.py Outdated Show resolved Hide resolved

sansyrox reviewed Aug 8, 2023

View reviewed changes

SuelenKarbivnychyy added 6 commits August 12, 2023 20:38

delete extra test

0c2f1bb

feat(dom_methods.py,-store.fyre): deleted <pyml> from store.fyre and …

956d54c

…updated the code to instead of raising a error when <slot> is not found, update the html, js and css anyway

refactor(test_quote-folder): its just a test folder for better explan…

7937443

…ation that will be deleted before merging the pr

refactor(dom_methods.py-parser.py-component.py): update the code stru…

c7ab9d5

…cture to follow design best practice implemented the code to make usage of children list instead of using a separate list to handle the custom component content we want to add to the slot

sansyrox force-pushed the sk-39 branch from c8b511c to c7ab9d5 Compare August 12, 2023 18:39

fix: parsing algo

b8d1008

sansyrox reviewed Aug 12, 2023

View reviewed changes

starfyre/parser.py Outdated Show resolved Hide resolved

fix(parser.py): added support for new self.current_children list

4d2ac7b

1. Put nodes into current_children

sansyrox reviewed Aug 18, 2023

View reviewed changes

starfyre/parser.py Outdated Show resolved Hide resolved

sansyrox reviewed Aug 18, 2023

View reviewed changes

starfyre/parser.py Outdated Show resolved Hide resolved

sansyrox reviewed Aug 18, 2023

View reviewed changes

starfyre/parser.py Outdated Show resolved Hide resolved

sansyrox reviewed Aug 18, 2023

View reviewed changes

starfyre/parser.py Show resolved Hide resolved

SuelenKarbivnychyy added 2 commits August 29, 2023 10:05

deleted empty spaces

ecf7897

fix(__init__.fyre): fixed a bug

6efec43

deleted extra <div> tag that didnt have closing tag for it.

sansyrox reviewed Aug 29, 2023

View reviewed changes

test: add a testing application

6ad7037

sansyrox force-pushed the sk-39 branch from 42263c4 to 6ad7037 Compare August 29, 2023 21:13

sansyrox requested changes Aug 29, 2023

View reviewed changes

SuelenKarbivnychyy added 2 commits August 30, 2023 19:08

Merge branch 'sk-39' of https://github.com/SuelenKarbivnychyy/starfyre …

5c8212e

…into sk-39 getting test code

fix(parser.py): Fixed order of slot compoments

bdd93d7

Fixed the text node order of slot components and implemented a approach to have the correct behavior when custom compoment is the root node in __innit__.fyre BREAKING CHANGE: sparckles#39

sansyrox reviewed Sep 1, 2023

View reviewed changes

sansyrox requested changes Sep 1, 2023

View reviewed changes

minor improvements

76a9921

update the algo

e3c29ce

sansyrox reviewed Sep 2, 2023

View reviewed changes

test-application/parent.fyre Outdated

<ul>

<li>a</li>

<li>a</li>

Copy link

Member

sansyrox Sep 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extra spaces

sansyrox reviewed Sep 2, 2023

View reviewed changes

test-application/parent.fyre Outdated

<li>b</li>

<li>c</li>

<li>c</li>

Copy link

Member

sansyrox Sep 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

sansyrox reviewed Sep 2, 2023

View reviewed changes

starfyre/parser.py Outdated Show resolved Hide resolved

add parser explanation

8e244a0

sansyrox reviewed Sep 2, 2023

View reviewed changes

docs(parser.py): added comments

7b6da80

included comments when processing data on parser

refactor(parser.py): added a TODO

3e9fb64

added a TODO to investigate why nested slot component is not rendered correct

sansyrox reviewed Sep 7, 2023

View reviewed changes

starfyre/parser.py Outdated Show resolved Hide resolved

Update starfyre/parser.py

8e8000d

sansyrox reviewed Sep 7, 2023

View reviewed changes

sansyrox merged commit 14500f4 into sparckles:main Sep 8, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Closes: #39 Add a way to specify the position of child components. #70

Closes: #39 Add a way to specify the position of child components. #70

SuelenKarbivnychyy commented Jul 27, 2023

SuelenKarbivnychyy commented Jul 27, 2023

sansyrox left a comment

SuelenKarbivnychyy commented Aug 1, 2023 •

edited

Loading

sansyrox left a comment

sansyrox left a comment

sansyrox commented Aug 12, 2023

SuelenKarbivnychyy commented Aug 28, 2023

sansyrox Aug 29, 2023

SuelenKarbivnychyy Aug 29, 2023

sansyrox left a comment

SuelenKarbivnychyy commented Aug 30, 2023

sansyrox Sep 1, 2023

sansyrox left a comment

SuelenKarbivnychyy commented Sep 1, 2023

sansyrox Sep 2, 2023

sansyrox Sep 2, 2023

sansyrox Sep 2, 2023

SuelenKarbivnychyy Sep 5, 2023

sansyrox Sep 5, 2023

sansyrox Sep 2, 2023

sansyrox commented Sep 5, 2023

SuelenKarbivnychyy commented Sep 5, 2023

sansyrox Sep 7, 2023

SuelenKarbivnychyy Sep 7, 2023

sansyrox left a comment

Closes: #39 Add a way to specify the position of child components. #70

Closes: #39 Add a way to specify the position of child components. #70

Conversation

SuelenKarbivnychyy commented Jul 27, 2023

SuelenKarbivnychyy commented Jul 27, 2023

sansyrox left a comment

Choose a reason for hiding this comment

SuelenKarbivnychyy commented Aug 1, 2023 • edited Loading

sansyrox left a comment

Choose a reason for hiding this comment

sansyrox left a comment

Choose a reason for hiding this comment

sansyrox commented Aug 12, 2023

SuelenKarbivnychyy commented Aug 28, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sansyrox left a comment

Choose a reason for hiding this comment

SuelenKarbivnychyy commented Aug 30, 2023

Choose a reason for hiding this comment

sansyrox left a comment

Choose a reason for hiding this comment

SuelenKarbivnychyy commented Sep 1, 2023

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

sansyrox commented Sep 5, 2023

SuelenKarbivnychyy commented Sep 5, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sansyrox left a comment

Choose a reason for hiding this comment

SuelenKarbivnychyy commented Aug 1, 2023 •

edited

Loading