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.

Sign up for GitHub

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

Jump to bottom

Persistence configuration page: Minor enhancements & fixes #1955

Merged
merged 8 commits into from
Jul 18, 2023
Merged

Persistence configuration page: Minor enhancements & fixes #1955

merged 8 commits into from
Jul 18, 2023

Conversation

florian-h05
Copy link
Contributor

@florian-h05 florian-h05 commented Jul 11, 2023 •
edited
Loading

Follow-up for #1917.

  • Display enabled filters in config footer.
  • Display filter configuration in filter footer.
  • Time filter: Define options for time unit.
  • Add a button to open the documentation.
  • Fixes an issue, where Filter creation fails due to missing filter type arrays.

@florian-h05 florian-h05 requested a review from a team as a code owner July 11, 2023 09:48
@florian-h05 florian-h05 added enhancement New feature or request main ui Main UI labels Jul 11, 2023
@florian-h05 florian-h05 added this to the 4.0 milestone Jul 11, 2023
@relativeci
Copy link

relativeci bot commented Jul 11, 2023 •
edited
Loading

Job #1069: Bundle Size — 15.75MiB (0%).

f6854e0(current) vs 17e6360 main#1068(baseline)

⚠️ Bundle contains 16 duplicate packages

Metrics (1 change)
                 Current
Job #1069		      Baseline
Job #1068
Initial JS 1.67MiB 1.67MiB
Initial CSS 608.89KiB 608.89KiB
Cache Invalidation 0% 93.94%
Chunks 219 219
Assets 689 689
Modules 1700 1700
Duplicate Modules 90 90
Duplicate Code 1.95% 1.95%
Packages 138 138
Duplicate Packages 15 15
Total size by type (no changes)
                 Current
Job #1069		      Baseline
Job #1068
CSS 859.34KiB 859.34KiB
Fonts 526.1KiB 526.1KiB
HTML 1.23KiB 1.23KiB
IMG 140.74KiB 140.74KiB
JS 9.24MiB 9.24MiB
Media 295.6KiB 295.6KiB
Other 4.72MiB 4.72MiB

View job #1069 report View main branch activity

@florian-h05 florian-h05 changed the title Persistence configuration page: Minor enhancements Persistence configuration page: Minor enhancements & fixes Jul 15, 2023
@florian-h05
Copy link
Contributor Author

@ghys Can you please review and merge this for openHAB 4? Probably to late for the feature freeze, but this PR contains a critical bug fix so IMO should be merged anyway.

ghys
ghys requested changes Jul 16, 2023
View reviewed changes
Copy link
Member

@ghys ghys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two minor remarks on the docs link, the rest LGTM.

bundles/org.openhab.ui/web/src/pages/settings/persistence/persistence-edit.vue Outdated
Comment on lines 167 to 169
<f7-list-button color="blue" :href="`https://${$store.state.runtimeInfo.buildString === 'Release Build' ? 'www' : 'next'}.openhab.org/docs/configuration/persistence.html`" external target="_blank">
Open documentation
</f7-list-button>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A list button is not the best for linking to the docs IMHO, those are better suited for internal actions. For those concepts that are a little obscure and need a link to the docs a f7-link labeled "Learn more [about X]" or "About X" would be better, see the profile configuration for example (the blurb before the link can be optional):

openhab-webui/bundles/org.openhab.ui/web/src/pages/settings/things/link/link-edit.vue

Lines 49 to 53 in 9be4293

<f7-block-footer class="padding-left padding-right">
Profiles define how Channels and Items work together. Install transformation add-ons to get additional profiles.
<f7-link external color="blue" target="_blank" href="https://www.openhab.org/link/profiles">
Learn more about profiles.
</f7-link>

or iOS settings (note how the list buttons does something while the documentation links are simply blue hyperlinks):

image

Also, as shown above for profiles (the www/next switch is missing there, sadly!) it would be better to create a openhab.org/link/persistence alias link or similar, that way if the docs are reorganized it wouldn't require a change in the code but only an update to the redirect. This happens here: https://github.com/openhab/website/blob/main/.vuepress/_redirects

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added a short paragraph with the link to the top and created a PR for the redirect, thanks for your feedback, looks much better now!

See openhab/website#413.

florian-h05 added a commit to florian-h05/website that referenced this pull request Jul 18, 2023
@florian-h05
Add redirect for persistence configuration docs
1f3b5b3 
Required for openhab/openhab-webui#1955.

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
@florian-h05 florian-h05 mentioned this pull request Jul 18, 2023
Merged
@florian-h05
Persistence page: Display filters for configs
38c053e 
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
@florian-h05
Persistence page: Display filter configuration as footer
aa52ca7 
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
@florian-h05
Persistence page: Display link to documentation
a36fc4f 
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
@florian-h05
Persistence page: Define options for time filter unit
ac40c1f 
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
@florian-h05
Persistence page: Remove save checks as core checks data
0a7faf3 
Depends on openhab/openhab-core#3681.

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
@florian-h05
Revert "Persistence page: Remove save checks as core checks data"
3a693d6 
This reverts commit 9def78f.

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
@florian-h05
Persistence page: Improve already existing checks & Create filterType…
299621b 
… arrays on load

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
@florian-h05
Address review
91f7d0a 
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
ghys pushed a commit to openhab/website that referenced this pull request Jul 18, 2023
@florian-h05
Add redirect for persistence configuration docs (#413)
9038cd2 
Required for openhab/openhab-webui#1955.

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
ghys
ghys approved these changes Jul 18, 2023
View reviewed changes
Copy link
Member

@ghys ghys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@ghys ghys merged commit 6a35b6c into openhab:main Jul 18, 2023
@florian-h05 florian-h05 deleted the persistence branch July 18, 2023 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ghys ghys ghys approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request main ui Main UI
Projects
None yet
Milestone
4.0
Development

Successfully merging this pull request may close these issues.
2 participants
@florian-h05 @ghys