show tags in monitor select list under status page #2752

titanventura · 2023-02-09T18:22:56Z

⚠️⚠️⚠️ Since we do not accept all types of pull requests and do not want to waste your time. Please be sure that you have read pull request rules:
https://github.com/louislam/uptime-kuma/blob/master/CONTRIBUTING.md#can-i-create-a-pull-request-for-uptime-kuma

Tick the checkbox if you understand [x]:

I have read and understand the pull request rules.

Description

Fixes #2739

Type of change

Please delete any options that are not relevant.

User interface (UI)

Checklist

My code follows the style guidelines of this project
I ran ESLint and other linters for modified files
I have performed a self-review of my own code and tested it
I have commented my code, particularly in hard-to-understand areas
(including JSDoc for methods)
My changes generate no new warnings
My code needed automated testing. I have added them (this is optional task)

Screenshots (if any)

Please do not use any external image service. Instead, just paste in or drag and drop the image here, and it will be uploaded automatically.

khanh96le · 2023-02-13T04:44:42Z

src/pages/StatusPage.vue

+                            <option v-for="monitor in allMonitorList" :key="monitor.id" :value="monitor">
+                                {{
+                                    monitor.name
+                                }}
+                                <span v-if="monitor.tags.length > 0">
+                                    [Tags: {{ monitor.tags.map(tag => tag.name).join(" ,") }} ]
+                                </span>
+                            </option>


Can you drop a screenshot of how does this change make on the UI?

sure, it looks like this

Thank you, I think that's all we need for the functionality.

Just a minor suggestion for the UI, do you think it would look better if we could display the tags as labels with color like this? We can get the color from the tag.color attribute.

In that case, i think it is better if we implement vue-multiselect as the component instead of using a plain select. because a plain select does not render colors consistently. For example, In the above screenshot (mac os), you can see that the color will probably not be visible since it is using the styles provided by the host operating system (mac os in this case)

ping @khanh96le

Please validate the above idea @khanh96le so that I can know if I can start to work on refactoring the PR. Thanks !

Hi @titanventura,

I am sorry for the late response.

I really appreciate the idea you proposed, it looks great to me! When you have a moment, can you share another screenshot of the updated version? I'm excited to see how it looks and I'm sure everyone else is too.

Thank you so much for your hard work and contributions!

Hi @khanh96le, no probs. thank you so much for taking the time to analyse and suggest. I have updated the select monitor component in edit status page. It uses vue-multiselect and shows tags with name and value (if present) with the background color of the tag. It looks like this.

Please do review and suggest changes if any. Thanks !

BTW, the e2e tests seem to be flaky. dddc239 passed but 388da6f failed

chakflying · 2023-02-18T17:25:17Z

It would be better to use the existing Tag.vue component instead of duplicating the styling code.

titanventura · 2023-02-18T18:12:09Z

sure @chakflying, thanks for the suggestion. Have made the changes !

titanventura · 2023-02-20T16:36:21Z

Ping @chakflying
@khanh96le

chakflying · 2023-02-20T18:36:15Z

Since you aren't adding new packages, you can remove package-lock from the commits so it won't show up here. (I usually use rebase to fix it)

titanventura · 2023-02-22T09:17:04Z

I've removed the package-lock.json file. Please do review and suggest changes. Thanks !

chakflying · 2023-02-23T11:24:21Z

I got error when I tried to run this. Do you know what's the issue?

Uncaught (in promise) TypeError: monitor is undefined
    option StatusPage.vue:347
    renderFnWithContext runtime-core.esm-bundler.js:853
    renderSlot runtime-core.esm-bundler.js:2968
    default vue-multiselect.esm.js:1279
    renderList runtime-core.esm-bundler.js:2884
    default vue-multiselect.esm.js:1262

titanventura · 2023-02-25T01:29:55Z

I guess this was a bug. Resolved it. I used the word monitor instead of option inside vue-multiselect. not sure why it didn't work with monitor though !

chakflying · 2023-02-26T09:27:58Z

Looks like the Tag component is not imported correctly so this still doesn't work. Don't want to be mean but looks like you did not test this before marking as ready.

Also recommend doing an actual rebase since master has the fix for the marked() issue, and you are now modifying the package-lock.json file twice instead of not touching it.

louislam · 2023-02-26T10:04:20Z

Also there are a lot of unrelated code changes. Please revert those changes. I guess you were using a incorrect eslint config.

And if you are not sure how to load the eslint config, you can use npm run lint-fix:style and npm run lint-fix:js

… from normal select to vue-multiselect

titanventura · 2023-02-26T10:57:56Z

Deleted the branch, created a new one and made changes only to StatusPage.vue. Works for me locally. Please do check if it works for you this time. Thanks for the suggestions !

titanventura · 2023-03-29T17:56:13Z

can we merge this PR ?

…-page-monitor-select-list # Conflicts: # src/pages/StatusPage.vue

titanventura mentioned this pull request Feb 9, 2023

Show tags on the dropdown list when adding a new monitor to a status page #2739

Closed

1 task

khanh96le reviewed Feb 13, 2023

View reviewed changes

louislam added this to the 1.22.0 milestone Feb 20, 2023

titanventura force-pushed the show-tags-in-status-page-monitor-select-list branch from e5eb836 to 948ff51 Compare February 22, 2023 07:00

show tags in monitor select list under status page : change select UI…

db6b863

… from normal select to vue-multiselect

titanventura force-pushed the show-tags-in-status-page-monitor-select-list branch from b5599f3 to db6b863 Compare February 26, 2023 10:56

chakflying approved these changes Mar 19, 2023

View reviewed changes

louislam added 2 commits April 4, 2023 15:35

Merge remote-tracking branch 'origin/master' into show-tags-in-status…

9dd1b1c

…-page-monitor-select-list # Conflicts: # src/pages/StatusPage.vue

Minor

7706c29

louislam merged commit be7d3f6 into louislam:master Apr 4, 2023

chakflying mentioned this pull request Apr 4, 2023

UI: Use vue-multiselect in Edit Tag & Styling Fixes #3024

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

show tags in monitor select list under status page #2752

show tags in monitor select list under status page #2752

titanventura commented Feb 9, 2023 •

edited by CommanderStorm

Loading

khanh96le Feb 13, 2023

titanventura Feb 13, 2023

khanh96le Feb 13, 2023

titanventura Feb 13, 2023

titanventura Feb 14, 2023

titanventura Feb 16, 2023

khanh96le Feb 17, 2023

titanventura Feb 18, 2023

titanventura Feb 18, 2023 •

edited

Loading

chakflying commented Feb 18, 2023

titanventura commented Feb 18, 2023

titanventura commented Feb 20, 2023 •

edited

Loading

chakflying commented Feb 20, 2023 •

edited

Loading

titanventura commented Feb 22, 2023

chakflying commented Feb 23, 2023 •

edited

Loading

titanventura commented Feb 25, 2023 •

edited

Loading

chakflying commented Feb 26, 2023

louislam commented Feb 26, 2023

titanventura commented Feb 26, 2023

titanventura commented Mar 29, 2023

show tags in monitor select list under status page #2752

show tags in monitor select list under status page #2752

Conversation

titanventura commented Feb 9, 2023 • edited by CommanderStorm Loading

Description

Type of change

Checklist

Screenshots (if any)

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

titanventura Feb 18, 2023 • edited Loading

Choose a reason for hiding this comment

chakflying commented Feb 18, 2023

titanventura commented Feb 18, 2023

titanventura commented Feb 20, 2023 • edited Loading

chakflying commented Feb 20, 2023 • edited Loading

titanventura commented Feb 22, 2023

chakflying commented Feb 23, 2023 • edited Loading

titanventura commented Feb 25, 2023 • edited Loading

chakflying commented Feb 26, 2023

louislam commented Feb 26, 2023

titanventura commented Feb 26, 2023

titanventura commented Mar 29, 2023

titanventura commented Feb 9, 2023 •

edited by CommanderStorm

Loading

titanventura Feb 18, 2023 •

edited

Loading

titanventura commented Feb 20, 2023 •

edited

Loading

chakflying commented Feb 20, 2023 •

edited

Loading

chakflying commented Feb 23, 2023 •

edited

Loading

titanventura commented Feb 25, 2023 •

edited

Loading