forked from tbnobody/OpenDTU
-
Notifications
You must be signed in to change notification settings - Fork 66
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 #1362
Merged
Merged
Webapp Polishing #1362
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
schlimmchen
commented
Oct 29, 2024
•
edited
Loading
edited
- 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).
- Fix the AC charger admin view.
- Always scroll up when navigating to another view.
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.
when using v-if on a div element, the div will be part of the DOM in case the condition is true. if we group elements to use v-if on the group, we shall use a template element, so the group elements appear as siblings of the other elements. in particular, these spurious div do not mess up our CSS patch that sets the bottom margin for the last child in a card to "auto".
avoid hidden (but existing) or simply redundant DOM elements from messing with the style sheet, which uses :last-child in particular to fix up the margin on the bottom of cards.
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.
avoids inline style and removes right padding such that the value and its unit move closed together, replicating the design of the inverter channel info tables.
the total cards have been trimmed at the bottom as the card's last child has its margin-bottom removed (set to "auto"). that is desired as it places the text in the middle of the cards. however, the battery total cards are different, as they show two values each, which are arranged by div.flex-fill containers, which are the children of the cards rather than the h2 tags as in all other totals cards.
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.
@tbnobody I am going to rebase this onto your master branch and create a pull request so you can pick what you like. I made small commits to allow picking partial changes, even though some build on others. |
Looks great :) |
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.
use InputElement where possible, which in particular fixes that the inputs of the second card were all in the same row.
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
force-pushed
the
webapp-polishing
branch
from
October 29, 2024 13:49
1d83f87
to
af8bd9f
Compare
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. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.