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

Redesign of Firmware section at Setup tab #3770

Merged
merged 18 commits into from Feb 16, 2024

Conversation

HThuren
Copy link
Member

@HThuren HThuren commented Jan 20, 2024

Changed according to request at discord
New design:
image

With local build and still logic if not online:
image

Press on Options give popup (possible to select text):
image

Old design:
image

This comment has been minimized.

@haslinghuis haslinghuis added this to the 10.10.0 milestone Jan 21, 2024
@ctzsnooze
Copy link
Member

ctzsnooze commented Jan 21, 2024

Looks really great, @HThuren ! Tested, works as proposed. Thank you!

The orange buttons are very intuitive and helpful.

Maybe the Defines Used button, currently a grey 'hover' button, could be an orange 'click to open in new window' button? I think it would be very intuitive like that. And easier to copy and paste the defines used, if needed.

Perhaps the 'help' text could then go back into the ? icon where it previously was, as a pop-up when hovering over the ?, like before?

@HThuren
Copy link
Member Author

HThuren commented Jan 21, 2024

@ctzsnooze thank you.
To change Options defined to orange button, a good idea, will require a popup window. Configurator only has such in preset tabs, and here quite sophiscated.
I'll construct a more 'general' popup with title, body and OK button. This can be in use in other future functionality.

@nerdCopter
Copy link
Member

IMHO, i prefer hover-over, not pop-out. sorry if i'm the only one.
I also prefer old tooltip-hover for description, not the new always-active text.

@haslinghuis
Copy link
Member

Hold fire on this PR as we are working on other methods to retrieve build options from firmware without the need for Build API queries as we need the options for LUA too.

@HThuren
Copy link
Member Author

HThuren commented Jan 22, 2024

right, anyway changed back to help in cf_help tooltip, as wanted from @nerdCopter, since the yellow note require lot of space.
Hover over are a possibility, but did look into a popup window a a general tool.
But halt both until @haslinghuis give green go

@haslinghuis
Copy link
Member

Just saw it yesterday as it's not my contribution - so improving this PR now does not hurt as we only attend to improve back-end call functions.

This comment has been minimized.

@ctzsnooze
Copy link
Member

If we keep the 'hover' behaviour, we should use the standard ? icon that we use for other hover elements.

A 'greyed out' button that doesn't work implies a 'normal' button that is disabled. It is bad UI design. The normal user might try to click it, just in case the colour loss was a mistake, but nothing happens, so that confirms their understanding that the button is disabled. They are likely to report this as a bug.

We don't have any other 'greyed out' button elements that mean 'hover over me' on any other windows.

A pop-out lets us copy the text, whereas a hover doesn't.

Anyhow, I still prefer an orange button with a pop-out, but if you want to keep it a hover, then use ? as the UI element, since people intuitively will hover over those.

PS There is another issue with the grey button hover, it is very slow to open, much slower than hovering over a ? element, which is nearly instantaneous. Hovers need to be instant otherwise they appear broken.

@HThuren
Copy link
Member Author

HThuren commented Jan 30, 2024

If we keep the 'hover' behaviour, we should use the standard ? icon that we use for other hover elements.

A 'greyed out' button that doesn't work implies a 'normal' button that is disabled. It is bad UI design. The normal user might try to click it, just in case the colour loss was a mistake, but nothing happens, so that confirms their understanding that the button is disabled. They are likely to report this as a bug.

We don't have any other 'greyed out' button elements that mean 'hover over me' on any other windows.

A pop-out lets us copy the text, whereas a hover doesn't.

Anyhow, I still prefer an orange button with a pop-out, but if you want to keep it a hover, then use ? as the UI element, since people intuitively will hover over those.

PS There is another issue with the grey button hover, it is very slow to open, much slower than hovering over a ? element, which is nearly instantaneous. Hovers need to be instant otherwise they appear broken.

I agree, the grey hover are different behaviour than elsewhere in BF.
I go for 3 orange buttons,
image
The '?' icon say
image

But are not done yet with at popup for "Options defined"

@nerdCopter
Copy link
Member

nerdCopter commented Jan 30, 2024

@HThuren , just a note, i'm a relative new-comer to betaflight team. my opinions should be regarded as my own, not the team's. other team members should have priority and say as well. that said, i've been on computers since 1985 and i think i have good feel for things.

edit: my concern with a pop-out was that such is a dialog that must be closed. this was my personal peeve. if there is a better way to do so, then i'm open. maybe it can auto-close if a user forgets it. 🤷‍♂️

@HThuren
Copy link
Member Author

HThuren commented Jan 30, 2024

@HThuren , just a note, i'm a relative new-comer to betaflight team. my opinions should be regarded as my own, not the team's. other team members should have priority and say as well. that said, i've been on computers since 1985 and i think i have good feel for things.

edit: my concern with a pop-out was that such is a dialog that must be closed. this was my personal peeve. if there is a better way to do so, then i'm open. maybe it can auto-close if a user forgets it. 🤷‍♂️

@nerdCopter no worry, I also use my own jugdement :-)
The popup are planning to be modal, then never to forget ..

@haslinghuis haslinghuis modified the milestones: 10.10.0, 11.0 Feb 4, 2024
@ctzsnooze
Copy link
Member

where are we at?

@HThuren HThuren requested review from ctzsnooze, chmelevskij and haslinghuis and removed request for chmelevskij February 11, 2024 19:56
@HThuren
Copy link
Member Author

HThuren commented Feb 11, 2024

Maybe part of 10.10 ?

This comment has been minimized.

This comment has been minimized.

@HThuren
Copy link
Member Author

HThuren commented Feb 14, 2024

I don't like it:
image

better without border and like
image

@haslinghuis
Copy link
Member

haslinghuis commented Feb 14, 2024

Need to define columns in the grid layout:

image

This comment has been minimized.

@HThuren
Copy link
Member Author

HThuren commented Feb 16, 2024

NOW done, use the more simple and dynamic grid. Support dark theme.

This comment has been minimized.

@nerdCopter
Copy link
Member

fd215243
image

flow gets wonky at come widths, but not very important:
wide:
image
1202px: (odd)
image
1164px: (okay by me)
image

@haslinghuis haslinghuis modified the milestones: 11.0, 10.10.0 Feb 16, 2024
src/js/tabs/setup.js Outdated Show resolved Hide resolved
Co-authored-by: Mark Haslinghuis <mark@numloq.nl>
Copy link

sonarcloud bot commented Feb 16, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

Copy link
Contributor

Do you want to test this code? Here you have an automated build:
Betaflight-Configurator-Android
Betaflight-Configurator-Linux
Betaflight-Configurator-macOS
Betaflight-Configurator-Windows
WARNING: It may be unstable and result in corrupted configurations or data loss. Use only for testing!

@haslinghuis haslinghuis merged commit c289445 into betaflight:master Feb 16, 2024
8 checks passed
@HThuren HThuren deleted the addQuestionMark branch February 16, 2024 15:19
chmelevskij pushed a commit to chmelevskij/betaflight-configurator that referenced this pull request Apr 27, 2024
* Redesign of Firmware section

* layout change

* popup work

* change branch

* more popup

* also main

* Final

* Final..

* removed popup.js - not needed

* with grid, missing center in frame

* missing center in frame

* Final

* Grid without table

* Fix quality gate issue

* Simple and synamic gris, and dark theme supported

* Update help

* Update help

* Update src/js/tabs/setup.js as suggested

Co-authored-by: Mark Haslinghuis <mark@numloq.nl>

---------

Co-authored-by: nerdCopter <56646290+nerdCopter@users.noreply.github.com>
Co-authored-by: Mark Haslinghuis <mark@numloq.nl>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: COMPLETED
Development

Successfully merging this pull request may close these issues.

None yet

4 participants