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

Allow custom Javascript files #2359

Merged
merged 14 commits into from
Dec 23, 2024

Conversation

namnguyen20999
Copy link
Member

What type of PR is this? (check all applicable)

  • Refactor
  • Feature
  • Bug Fix
  • Optimization
  • Documentation Update

Description

Allowing custom javascript file from GUI

Related Tickets & Documents

#1808

Checklist

We encourage you to keep the code coverage percentage at 80% and above.

  • This solution meets the acceptance criteria of the related issue.
  • The related issue checklist is completed.
  • This PR adds unit tests for the developed code. If not, why?
  • End-to-End tests have been added or updated. If not, why?
  • The documentation has been updated, or a dedicated issue created. (If applicable)
  • The release notes have been updated? (If applicable)

@namnguyen20999 namnguyen20999 linked an issue Dec 20, 2024 that may be closed by this pull request
6 tasks
@namnguyen20999 namnguyen20999 self-assigned this Dec 20, 2024
@namnguyen20999 namnguyen20999 added the 🖰 GUI Related to GUI label Dec 20, 2024
@namnguyen20999 namnguyen20999 changed the title initial commit Allow custom Javascript files Dec 20, 2024
Copy link
Contributor

github-actions bot commented Dec 20, 2024

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines Covered Coverage Threshold Status
19492 16981 87% 0% 🟢

New Files

No new covered files...

Modified Files

File Coverage Status
taipy/gui/gui.py 79% 🟢
TOTAL 79% 🟢

updated for commit: 6837925 by action🐍

Copy link
Member

@FabienLelaquais FabienLelaquais left a comment

Choose a reason for hiding this comment

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

Looks good to me

taipy/gui/gui.py Outdated Show resolved Hide resolved
taipy/gui/gui.py Outdated Show resolved Hide resolved
Copy link
Member

@FredLL-Avaiga FredLL-Avaiga left a comment

Choose a reason for hiding this comment

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

a few comments

Page specific script will be handled in another PR ?

taipy/gui/gui.py Outdated Show resolved Hide resolved
taipy/gui/gui.py Outdated Show resolved Hide resolved
taipy/gui/gui.py Outdated Show resolved Hide resolved
taipy/gui/gui.py Outdated Show resolved Hide resolved
taipy/gui/gui.py Outdated Show resolved Hide resolved
taipy/gui/gui.py Outdated Show resolved Hide resolved
taipy/gui/gui.py Outdated Show resolved Hide resolved
Copy link
Member

@FabienLelaquais FabienLelaquais left a comment

Choose a reason for hiding this comment

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

My guts tell me glob() is usually not so safe, portability-wise. But I assume tests ensure this works everywhere...

taipy/gui/gui.py Outdated
@@ -411,6 +419,32 @@ def __init__(
for library in libraries:
Gui.add_library(library)

def __load_scripts(self, script_paths: t.Optional[t.Union[str, Path, t.List[t.Union[str, Path]]]]):
def load_script(path: t.Union[str, Path]):
Copy link
Member

Choose a reason for hiding this comment

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

what's the point of inner defined function ?

Copy link
Member Author

Choose a reason for hiding this comment

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

it was based on Fabien suggestion

Copy link
Member

Choose a reason for hiding this comment

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

then ...

Copy link
Member

Choose a reason for hiding this comment

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

Inner functions don't clobber the namespace. I like that, as long as these functions are not evaluated over and over... which is the case here.

Copy link
Member

Choose a reason for hiding this comment

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

the code of the inner function is called only once ... what's the point ?

Copy link
Member

Choose a reason for hiding this comment

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

Oh... that's different.
A function called only once is kind of useless indeed.
Then a comment does the trick, expanding the function where it belongs.

Copy link
Member

@FabienLelaquais FabienLelaquais left a comment

Choose a reason for hiding this comment

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

👌

@namnguyen20999 namnguyen20999 merged commit 4153273 into develop Dec 23, 2024
126 checks passed
@namnguyen20999 namnguyen20999 deleted the 1808-allow-custom-javascript-files branch December 23, 2024 10:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🖰 GUI Related to GUI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow custom Javascript files
3 participants