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

Inline iro.js and rangetouch.js #3597

Merged
merged 6 commits into from
Dec 21, 2023
Merged

Inline iro.js and rangetouch.js #3597

merged 6 commits into from
Dec 21, 2023

Conversation

w00000dy
Copy link
Contributor

Since #3511 has been merged, we can now include iro.js and rangetouch.js using the inliner again.

I also removed the unused feedback function as it was probably forgotten to remove in commit d6c0642.

Copy link
Collaborator

@blazoncek blazoncek left a comment

Choose a reason for hiding this comment

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

Generally approved from my side.
Removal of iro.js and rangetouch.js was needed for simplified UI.

You may want to edit cdata.json to comment out generation of .htm files and remove/comment references from wled_server.cpp for PR to be complete.

@@ -7,52 +7,9 @@
<meta content="yes" name="apple-mobile-web-app-capable">
<link rel="shortcut icon" href="data:image/x-icon;base64,AAABAAEAEBAAAAEAGACGAAAAFgAAAIlQTkcNChoKAAAADUlIRFIAAAAQAAAAEAgGAAAAH/P/YQAAAE1JREFUOI1j/P//PwOxgNGeAUMxE9G6cQCKDWAhpADZ2f8PMjBS3QW08QK20KaZC2gfC9hCnqouoNgARgY7zMxAyNlUdQHlXiAlO2MDAD63EVqNHAe0AAAAAElFTkSuQmCC"/>
<title>WLED</title>
<script>
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would keep this for future reference as a comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see no advantage in keeping that as a comment. If you want to look at the code again in the future, you can simply look at the Git history.
I think that would just make the code less readable and most people would probably wonder what this comment is for when they see it.

And what exactly would you want to leave as a comment? The complete <script> block or just the feedback function?

Copy link
Collaborator

Choose a reason for hiding this comment

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

History is one thing, actual snippet embedded as a comment is another. Snippet can be used immediately by anyone while code from history is only viable option for those that know it was there. Snippet (as a comment) does not reduce readability of the code, even without any actual comment in it, on the contrary IMO it adds value for a possible future when such feature may be re-implemented.

Dynamically loading of JS files may come in handy in the future (for possible extensions) and at least POC code should be kept in place as a guideline for such case. Commented code does not increase binary code size or adds performance penalties.

I am in favour of reducing HTTP requests to ESP but not at the expense of removing knowledge from source files. Otherwise you can strip every comment from all source files.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a comment with referencte to how it was done in 0.14.0. What do you think of this?
I think this is better because we don't have this big block of commented out code (so the code looks cleaner) and we still preserve the knowledge.

@blazoncek blazoncek merged commit 6c877b6 into wled:0_15 Dec 21, 2023
12 checks passed
w00000dy added a commit to w00000dy/WLED that referenced this pull request Jan 3, 2024
commit bd20c83
Author: Blaz Kristan <blaz@kristan-sp.si>
Date:   Mon Jan 1 23:14:45 2024 +0100

    Bugfix wled#3632

commit 48f8a45
Author: Blaz Kristan <blaz@kristan-sp.si>
Date:   Sun Dec 31 18:05:21 2023 +0100

    Last minute adjustments & fixes

commit 680cec5
Author: Istvan Meszaros <contact.imeszaros@gmail.com>
Date:   Tue Dec 26 11:09:20 2023 +0100

    Make palette editor mobile friendly.

commit ecdc3be
Author: Blaz Kristan <blaz@kristan-sp.si>
Date:   Sat Dec 30 11:56:21 2023 +0100

    Changlog update

commit 301bdf2
Merge: 1c1b67e d3be7a3
Author: Blaž Kristan <blaz@kristan-sp.si>
Date:   Sat Dec 30 11:00:53 2023 +0100

    Merge pull request wled#3610 from Aircoookie/psram-4-json

    Allow PSRAM (SPI RAM) to be used for JSON buffer

commit 1c1b67e
Author: Blaz Kristan <blaz@kristan-sp.si>
Date:   Fri Dec 29 23:07:29 2023 +0100

    Full per-port ABL implementation and bugfix.
    Update of BusManager class (static)

commit d3be7a3
Merge: 7971f3c 6cd0da8
Author: Blaz Kristan <blaz@kristan-sp.si>
Date:   Thu Dec 28 23:38:27 2023 +0100

    Merge branch '0_15' into psram-4-json

commit 6cd0da8
Author: Blaz Kristan <blaz@kristan-sp.si>
Date:   Thu Dec 28 23:32:47 2023 +0100

    Reduce heap fragmentation when switching ledmaps

commit 6332ee6
Author: Blaz Kristan <blaz@kristan-sp.si>
Date:   Wed Dec 27 19:36:25 2023 +0100

    Effect buffer optimisations
    setMode() fix for selecting gap
    UI error messages

commit 7971f3c
Author: Blaz Kristan <blaz@kristan-sp.si>
Date:   Sat Dec 23 22:56:07 2023 +0100

    Fix for saving config.

commit 08d9f7d
Author: Blaz Kristan <blaz@kristan-sp.si>
Date:   Sat Dec 23 21:05:01 2023 +0100

    Fix for wled#2922

commit 776502e
Merge: 4e10549 7af5b24
Author: Blaž Kristan <blaz@kristan-sp.si>
Date:   Sat Dec 23 20:27:20 2023 +0100

    Merge pull request wled#3606 from WoodyLetsCode/welcome

    Fix bug that welcome page was not shown

commit 7af5b24
Author: Woody <xaver@holzapfel.net>
Date:   Thu Dec 21 22:17:43 2023 +0100

    Fix bug that welcome page was not shown

commit 1f81fb9
Author: Blaz Kristan <blaz@kristan-sp.si>
Date:   Thu Dec 21 21:30:17 2023 +0100

    Implement JSON buffer in PSRAM to free up DRAM.

commit 4e10549
Author: Blaz Kristan <blaz@kristan-sp.si>
Date:   Thu Dec 21 21:02:53 2023 +0100

    Retry on failed JSON load.
    Throttle for ESP8266
    This is a band-aid for ESP8266 JSON corruption and not a proper fix.

commit abf7dd5
Merge: 6c877b6 37b8072
Author: Blaž Kristan <blaz@kristan-sp.si>
Date:   Thu Dec 21 20:49:44 2023 +0100

    Merge pull request wled#3587 from WoodyLetsCode/search-scroll-top

    Scroll to first search result on search

commit 6c877b6
Merge: 42dba31 86d2998
Author: Blaž Kristan <blaz@kristan-sp.si>
Date:   Thu Dec 21 20:43:29 2023 +0100

    Merge pull request wled#3597 from WoodyLetsCode/inline

    Inline iro.js and rangetouch.js

commit 86d2998
Author: Xaver <xaver@holzapfel.net>
Date:   Wed Dec 20 11:30:28 2023 +0100

    Add reference to old loading of iro.js and rangetouch.js

commit 201daf8
Author: Woody <xaver@holzapfel.net>
Date:   Tue Dec 19 22:40:24 2023 +0100

    Remove /iro.js and /rangetouch.js

commit 1140f5f
Author: Woody <xaver@holzapfel.net>
Date:   Tue Dec 19 21:18:33 2023 +0100

    Update version to 2312190

commit 0a91d60
Author: Woody <xaver@holzapfel.net>
Date:   Tue Dec 19 21:12:07 2023 +0100

    add onload event to body

commit 5cd8f56
Author: Woody <xaver@holzapfel.net>
Date:   Tue Dec 19 20:41:56 2023 +0100

    Remove unused feedback function
    This was probably forgotten to remove in commit d6c0642

commit b2babd9
Author: Xaver <xaver@holzapfel.net>
Date:   Tue Dec 19 16:52:15 2023 +0100

    inline iro.js and rangetouch.js

commit 37b8072
Author: Woody <xaver@holzapfel.net>
Date:   Wed Dec 13 18:22:09 2023 +0100

    Scroll to first search result on search
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants