chore: Remove erb templete requirments from editor#2964
Conversation
arkirchner
force-pushed
the
ak/remove_some_js_erb
branch
from
d3f2a89 to
51e2447
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2964 +/- ##
==========================================
- Coverage 69.60% 69.55% -0.05%
==========================================
Files 214 214
Lines 6820 6823 +3
==========================================
- Hits 4747 4746 -1
- Misses 2073 2077 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
MrSerth
left a comment
MrSerth
left a comment
There was a problem hiding this comment.
I already made a few comments before you switched to the draft, so please find them below.
Thanks for working on moderninzing the assets. I think the initiative is good, but would also recommend on focussing on some aspects before opening up too many topics and PRs in parallel.
arkirchner
force-pushed
the
ak/remove_some_js_erb
branch
from
51e2447 to
16380ef
Compare
Configurations can be suppied to JavaScript with data attributes. This simplifies the JavaScript migration to Webpack.
arkirchner
force-pushed
the
ak/remove_some_js_erb
branch
from
16380ef to
3599da6
Compare
arkirchner
force-pushed
the
ak/remove_some_js_erb
branch
from
106fffc to
f14cdff
Compare
| @@ -1,5 +1,5 @@ | |||
| doctype html | |||
| html lang=I18n.locale data-default-locale=I18n.default_locale | |||
| html lang=I18n.locale *html_data_attributes | |||
There was a problem hiding this comment.
After we got the other parts right here, I am returning to my previous question regarding the gloabl scope.
Previously, you wrote:
The JavaScript needs this when the page loads. I don't want to make too large changes to JavaScript at the moment.
Some of the data attributes are not needed when the page loads, but can also be parsed later, I'd say. Rather than touching this part multiple times, I would suggest to get it right now.
|
Closing this because the complexity has exploded through the reviews. |
I am really surprised by this conclusion, and think we got a good progress already. Since we all spent some time already, the migration is needed (and you chose to work on this topic), I would be in favor of finalizing it. From my side, there is a single comment left right now, specifically the scope of the attributes. @nenock Can you help out, please? |
|
We finished the migration with #2997 (and openHPI/codeharbor#2059 for CodeHarbor). Therefore, I am removing the old branch. |
2 participants
Configurations can be suppied to JavaScript with data attributes. This simplifies the JavaScript migration to Webpack.