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 > 3 digits in numerical entry #1943

Open
davidperrenoud opened this issue Aug 25, 2014 · 11 comments
Open

allow > 3 digits in numerical entry #1943

davidperrenoud opened this issue Aug 25, 2014 · 11 comments
Milestone

Comments

@davidperrenoud
Copy link
Contributor

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
Copy link
Contributor

failiz commented Nov 17, 2020

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

@failiz
Copy link
Contributor

failiz commented Apr 25, 2021

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 added a commit to failiz/fritzing-app that referenced this issue Aug 12, 2021
…. Capacitors and resistors Allows intruducing up top 10 digits (fixes fritzing#1943)
@failiz
Copy link
Contributor

failiz commented Jul 26, 2022

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
Copy link
Contributor

failiz commented Jul 26, 2022

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);

hsoj pushed a commit to hsoj/fritzing-app that referenced this issue Aug 5, 2022
…. Capacitors and resistors Allows intruducing up top 10 digits (fixes fritzing#1943)
@failiz
Copy link
Contributor

failiz commented Nov 8, 2022

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

@KjellMorgenstern
Copy link
Member

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
Copy link
Contributor

failiz commented Nov 11, 2022

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
Copy link
Member

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
Copy link
Contributor

failiz commented Nov 14, 2022

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
Copy link
Contributor

failiz commented Jul 13, 2023

@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

@KjellMorgenstern KjellMorgenstern modified the milestones: 1.0.0, next Jul 13, 2023
@failiz
Copy link
Contributor

failiz commented Sep 20, 2023

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).

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

No branches or pull requests

4 participants