-
Notifications
You must be signed in to change notification settings - Fork 162
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
Moving to BS5 with a bit of 3rd party updating #531
base: main
Are you sure you want to change the base?
Conversation
Addressing : #499 |
Addressing: #452 |
Addressing : #431 |
Addressing: #448 |
59 test failures: https://github.com/Pylons/deform/actions/runs/6956721991/job/18928147423?pr=531#step:7:6539. See https://github.com/Pylons/deform/blob/main/contributing.md#when-to-add-or-edit-functional-tests for how to fix the tests. |
I'm currently trying to touch a bit of everything in a "do it first then fix it" kind of way. I'll take care of the tests once we're happy with the fixes. This PR is by no means complete, it allows us to possibly communicate of the different issues. Sorry for the noise, if it produces any. |
…, regarding errors and required
…Same attempt wit the datetime input, where required and error were added
Ah, OK, in that case, I converted this PR to Draft status, which implies, "Please don't review yet, I'm working on it." When ready, you can either |
The upload widget is flat out not working, because of the javascript behind it. This needs some heavier work. |
Starting working on the functional tests. They need a bit of love since the introduction of the html5 "required" changes a lot of the validation tests. |
@trollfot maybe it helps i had the same issue in the function test and solved it my manually scrolling a bit https://github.com/Pylons/deformdemo/pull/128/files#diff-e1608f44e2798812f15d2c4a50224a8eabd5e8604a8bffbf981523345ae6e130R3369 |
Indeed, they should be translatable. BTW, I really appreciate all the work y'all are putting into this PR. I've been swamped with $WORK, and have a couple more weeks until I can adequately address progress and carve out time to make that alpha release I've been promising. |
@delijati What issue are you talking about ? The code you referenced is already there, since it's based on the min branch where your code was already merged. Am I missing something ? |
this branch here aka this test is failing https://github.com/Pylons/deform/actions/runs/7199539548/job/19611426675?pr=531#step:7:607 the link i posted was just an example on how the test could be fixed |
@delijati The failing test is "FAILED deformdemo/test.py::DelayedRichTextWidgetTests::test_submit_filled"n due to the update of TinyMCE. I guess i'm totally missing the point |
No problem, the tinymce test is failing because tinymce is not in "view":
What i'm suggesting is a fix i did on some other tests to scroll manually into view
|
@trollfot do you have some time to look into the remaining issues? |
@delijati I tried the method you gave me but I couldn't get the test to pass. I think it's the last problem |
Ok i will also try to find some time to look into it :) |
@delijati I have time for this, we really want it merged and moving forward |
Hi i got the last functional test [1] to work. The problem is that the iframe inside of tinymce has a height of This is also not really perfect as the text in the textarea will be shown with its html tags. Also do we really need this feature? [1] error
[2] deformdemo diff --git a/deformdemo/test.py b/deformdemo/test.py
index ab4bfa2..c6eaca6 100644
--- a/deformdemo/test.py
+++ b/deformdemo/test.py
@@ -3398,7 +3398,7 @@ class DelayedRichTextWidgetTests(Base, unittest.TestCase):
url = test_url("/delayed_richtext/")
def test_submit_filled(self):
- findcss(".tinymce-preload").click()
+ findcss(".tinymce").click()
time.sleep(0.5)
browser.switch_to.frame(browser.find_element(By.TAG_NAME, "iframe"))
ActionChains(browser).scroll_by_amount(0, 200).perform() [3] deform diff --git a/deform/templates/richtext.pt b/deform/templates/richtext.pt
index 63fca94..dd17a54 100644
--- a/deform/templates/richtext.pt
+++ b/deform/templates/richtext.pt
@@ -7,13 +7,6 @@
i18n:domain="deform"
tal:omit-tag="">
- <style type="text/css">
- .deform .tinymce-preload{
- border: 1px solid #CCC;
- height: 240px;
- display: block;
- }
- </style>
<textarea id="${oid}" name="${name}"
tal:content="cstruct"
class="tinymce form-control ${field.error and error_class or ''}" />
@@ -22,9 +15,6 @@
that might result in non-HTML-escaped interpolation of
user input.
-->
- <span id="${oid}-preload" class="tinymce-preload"
- tal:condition="delayed_load and not field.error"
- tal:content="structure cstruct" />
<script type="text/javascript">
(function($){
deform.addCallback('${oid}', function(oid) {
@@ -35,16 +25,12 @@
selector: '#' + oid
};
var jqoid = $('#' + oid);
- var jqoid_preload = $('#' + oid + '-preload');
- if (jqoid_preload.length == 0) {
- tinyMCE.init(options);
- } else {
- jqoid.hide();
- jqoid_preload.one('click', function(){
- jqoid.show();
- jqoid_preload.remove();
+ if ('${delayed_load and not field.error}' == 'True') {
+ jqoid.one('click', function(){
tinyMCE.init(options);
});
+ } else {
+ tinyMCE.init(options);
}
});
$().bind('form.pre.serialize', function(event, $form, options) { |
Tests are green |
@stevepiercy Can we review this and get it moving ? |
I have setup a deform demo server so it is easier to cross check :) When we can merge i will stop the instance. NEW: OLD: |
@stevepiercy sorry to bother you but can we get a ETA when this can be reviewed and merged? |
Could we get the ball rolling ? It's been a while :) |
Is this dead, then ? |
@trollfot i think its as dead as we let it be ... my project where i use deform is currently being reassessed so there is a chance i need this updates ... |
This is a PR encompassing a few BS5 update for fields (classes) and a few upgrade of versions (tinyMCE, datepicker).
The PR is presented early to allow comments and change in direction.