fritzing / fritzing-app

Fritzing desktop application
http://fritzing.org
Other
3.98k stars 826 forks source link

allow > 3 digits in numerical entry #1943

Open davidperrenoud opened 10 years ago

davidperrenoud commented 10 years ago

From irasc...@gmail.com on April 10, 2012 18:15:12

Allow entries like 1000uF. Convert them directly to the correct prefix?

Original issue: http://code.google.com/p/fritzing/issues/detail?id=1943

failiz commented 3 years ago

It should not be difficult to implement if my pull request is accepted #3749

failiz commented 3 years ago

remove imported.

3749 is also included in the simulation branch, so this should be easy to fix when the code is merged into develop.

failiz commented 2 years ago

Hi @KjellMorgenstern , I noticed that you commented my change in capacitor.cpp to fix this issue: The code looks like:

//          QString pattern = QString("((\\d{0,10})|(\\d{0,10}\\.)|(\\d{0,10}\\.\\d{1,10}))[%1]{0,1}[%2]{0,1}")
            QString pattern = QString("((\\d{1,3})|(\\d{1,3}\\.)|(\\d{1,3}\\.\\d{1,2}))[%1]{0,1}[%2]{0,1}").arg(
//          QString pattern = QString("((\\d{0,3})|(\\d{0,3}\\.)|(\\d{0,3}\\.\\d{1,3}))[%1]{0,1}[%2]{0,1}")

The first line (and commented) would allow having 10 digits in the entry and it will also allow deleting all the numbers. What is the issue?

failiz commented 2 years ago

In fact, I think the pattern is the same as in Resistor.cpp:

QString pattern = QString("((\\d{0,10})|(\\d{0,10}\\.)|(\\d{0,10}\\.\\d{1,10}))[%1]{0,1}[%2]{0,1}")
                                          .arg(TextUtils::PowerPrefixesString)
                                          .arg(OhmSymbol);
failiz commented 1 year ago

? It would be nice to fix this in next version...

KjellMorgenstern commented 1 year ago

Sorry, that probably was a issue when merging commits. I have modified it to your suggestion:

QString pattern = QString("((\\d{0,10})|(\\d{0,10}\\.)|(\\d{0,10}\\.\\d{1,10}))[%1]{0,1}[%2]{0,1}")
                    .arg(TextUtils::PowerPrefixesString, propertyDef->symbol);

When testing, I noticed the physical unit sometimes (randomly?) changes to mF. This is not related to the 3 vs 10 digit limit.

failiz commented 1 year ago

Excellent. Thanks! Regarding the randomly changes that you experienced, did you introduce out of range values? If so, the fixup function of the validator reduces (or increases) the value until it's acceptable. Maybe, it could be better to do not modify the value at all.

KjellMorgenstern commented 1 year ago

Not sure what is broken... what the code intended was, if you enter for example "10000nF" it will be automatically changed to "10pF". So actually, one would never need more than 3 main digits.

failiz commented 1 year ago

Capacitors have a maximum capacitance of 4.7mF. If you introduce anything else above that threshold, the fixup function will be activated when (1) you press enter or (2) the focus is lost. I made the function to decrease or increase the value to achieve an ok value. For example, if you introduce "5", it will divide by ten until achieve something in the appropriate range, [1pF to 4.7mF]. Thus, you will end up with 0.5F. Apart from that, the code use the TextUtils::convertToPowerPrefix to format it to the appropriate format (3 main digits). If the value is in the correct range, e.g., "10000nF", the text is not modified.

Maybe an improvement would be to: (1) not attempt to fixup the text (if it is invalid, do not modify the value of the capacitor) and (2) format the text to use 3 main digits.

failiz commented 1 year ago

@KjellMorgenstern , could you reopen this? Add a capacitor in 1.0.0 and try to add more than 3 digits, it is impossible. In addition, I noticed that the background cells are not colored (green, red, grey) based on the current value. I am not sure if this is deliberated, but the help or hint still mentions the background colors. image

failiz commented 1 year ago

I checked and the function textModified in capacitor.cpp is being called, but the background color of the cell does not change. Any ideas why?

void Capacitor::textModified(QValidator::State state) {
    BoundedRegExpValidator * validator = qobject_cast<BoundedRegExpValidator *>(sender());
    if (validator == NULL) return;
    FocusOutComboBox * focusOutComboBox = qobject_cast<FocusOutComboBox *>(validator->parent());
    if (focusOutComboBox == NULL) return;

    if (state == QValidator::Acceptable) {
        QColor backColor = QColor(210, 246, 210);
        QLineEdit *lineEditor = focusOutComboBox->lineEdit();
        QPalette pal = lineEditor->palette();
        pal.setColor(QPalette::Base, backColor);
        lineEditor->setPalette(pal);
    } else if (state == QValidator::Intermediate) {
        QColor backColor = QColor(246, 210, 210);
        QLineEdit *lineEditor = focusOutComboBox->lineEdit();
        QPalette pal = lineEditor->palette();
        pal.setColor(QPalette::Base, backColor);
        lineEditor->setPalette(pal);
    }
}

In addition, I noticed a few months ago a weird issue when the symbol contains the letter x (I discovered this when preparing the photo-resistor which has a property called "lx"). Then, the validator fails. Tru to introduce 1lx, you are not able to introduce the x. My solution was to remove the symbol, called the validator and introduce the symbol again. This change is lost. The last commit removed this code (which had a bug, the idea was to remove the symbol, call the validator and put the symbol again, so the order in the picture is wrong). I had another commit where I fixed this, but it was not merged in 1.0.0 image

This code, in addition, makes sure that the symbol is always present in the property, which I think is a nice feature.

We should also change the fixup function to keep the last valid value and restore that value if the text introduced is not valid when the user presses enter or there is a focus-out event (instead of trying to fix the value).