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

Webapp Polishing #2388

Closed
wants to merge 28 commits into from
Closed

Conversation

schlimmchen
Copy link
Contributor

@schlimmchen schlimmchen commented Oct 29, 2024

  • Adjust the look of cards and accordion items with tables in them (in the live view and info views) to the look of inverter channel cards to achieve a consistent and condensed look.
  • Adjust spacing in a lot of places, mainly around forms.
  • Organize the pin mapping categories in cards to improve readability.
  • Move form labels to the right (on md and larger viewports) such that they "belong" to the input they describe.
  • Remove colons from labels, as some already did not use them (inconsistent) and because no table uses colons in headers as well, trying to achieve a cleaner look.
  • Move buttons to the right in general.
  • Avoid inline styles.
  • Avoid hidden elements (use v-if instead).
  • Always scroll up when navigating to another view.

localhost_8096_login

localhost_8096_home

localhost_8096_settings_network

localhost_8096_settings_device

localhost_8096_sys_info

Closes #2347.

this change tries to achieve a pleasing look of input forms by
right-aligning the texts of labels. the input form now looks similar
to a table, achieving a cleaner look, especially for forms where the
labels have varying text lenghts.
similar to the first row which has no border at the top.
we don't need a margin at the bottom of tables in general. not sure why
this is even a thing in bootstrap. this change, in particular, makes the
space between a table and a parent card symmetric on all sides.
this change adjusts the style of cards showing tables such that they
look the same as inverter channel info tables.
set the left margin of table header cells to the same marging the card
header use, such that the text align on the same axis.
the cards in all information views still used a div.card-body around the
table, which added a margin on all sides of the table. to achieve a
unified look, these cards and tables now look the same as the inverter
channel cards.
this is relevant for the radio statistics table, as well as the tables
in the grid profile modal.
it would be nice to have this in the header of the accordion, which is
hard, but doable. however, clicking the button then also toggles the
accordion, which is unacceptable. preventing that seems non-trivial, as
the @click.stop() is not enough. also, nesting interactive elements is
simply bad practice. the button can also go to the right of header, with
reasonable effort, but the corner radii are then messed up and would
need to react interactively (accordion collapsed or not), which is also
a pain.

we now "float" the reset button to the right, add a nice icon, and give
the button some space so it at least looks like it belongs there.
the source tells us that the buttons are supposed to be on the right of
tha card, but the CSS broke at some point.
if the last child in a card (div.card > div.card-body) adds bottom
marging, we don't want the card to add more space through its
padding-bottom. most cards have children that add sufficient space
at the bottom anyways.
if we hide elements (which is done using style="display:none;"), they
are still part of the DOM and mess with CSS rules that shall apply to
the last element of a card or the last row of a table.
in the settings view we hide the "login with cert" setting while TLS is
disabled, so we should also hide that info in the info view when TLS is
disabled.
there are no colons for table headers as well. some form labels had no
colon already, so this change uses a unified look among form labels.
* remove empty container for device profile links. if a device profile
  has no links, no buttons are generated, but a row was still part of
  the DOM, adding spurious space between the select and the alert with
  the hint.
* "float" the buttons to the right, as we always place these kinds of
  buttons to the right.
on a desktop browser, this approach allows to display all categories at
once. we also increase readability as the values are much closer to
their label. previously, the values were far to the right of the screen
and it was unpleasent to read which value belonged to which setting. the
grouping of values per category was also not very well conceived.

by using cards, we also avoid some styling issues, namely the use of
rowspan, which caused a spurious table cell border at the end of the old
table layout.
the top border of the card was breaking the design of the tabs, where
the active tab would be "visually connected" to the content. also, the
rounded border at the top did not blend in with the navbar's bottom
border.
this avoids the input text box from colliding with the tab navigation
bottom border.
improve spacing and align login buton to the right, where all our
buttons are.
long forms, when scrolled to the bottom, would leave no space between
the bottom of the viewport and the buttons, which is unpleasent.

short views would still createa large (high) body, for apparently no
reason.
@schlimmchen schlimmchen force-pushed the webapp-polishing-upstream branch from ec69660 to 696200a Compare October 29, 2024 13:39
on small viewports, the icon and the inverter label would be displayed
in two lines. this change keeps the icon and the label tied together in
any case, and the icon is centered vertically around the label.
fixes an annoying warning (visible in the browser console):

[Vue warn]: Extraneous non-emits event listeners (reload) were passed to
component but could not be automatically inherited because component
renders fragment or text root nodes. If the listener is intended to be a
component custom event listener only, declare it using the "emits"
option.
@schlimmchen
Copy link
Contributor Author

schlimmchen commented Nov 11, 2024

Thanks, @tbnobody, for considering all of the work in put into this 😊

When merging your latest tag I saw that you opted to place the documentation buttons in the device profile view to the left but aligned to the combo box. Makes sense and I merged your version of this change.

The diffs in the "optimize firmware update view" commit contained a card that is specific to the downstream project, which you cleaned up. Good catch. Sorry for not separating that out, that one slipped through.

You did not yet merge 3c0fabf (or 827a272 in the downstream development branch), but I guess that might simply be because I added it late to this PR and you were in the process of preparing the latest release already. As that change is not important for users, AFAIK, Nevermind, there it is 👍

I will close this PR now and consider it completed.

@schlimmchen schlimmchen deleted the webapp-polishing-upstream branch November 11, 2024 21:38
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new discussion or issue for related concerns.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ansicht bei mehreren Wechselrichter
1 participant