Closed cstns closed 2 days ago
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 78.75%. Comparing base (
698d94c
) to head (5b221c2
).
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
I ran into something strange and couldn't figure out why e2e tests are passing locally but failing in CI.
I finally got to replicate the CI result by running the npm run build
command before running the test suite as opposed to running npm run serve
locally.
Issue turned out to be caused by 'this', used in vue components, being evaluated so very slightly differently depending on the build mode you run the vue app in.
I had to remove the devices editor link last minute because the link would be disabled at all times due to missing device information that we were not receiving through application devices but through individual device api calls
I had to remove the devices editor link last minute because the link would be disabled at all times due to missing device information that we were not receiving through application devices but through individual device api calls
Does that mean we intend on shipping with out that and it will be a follow up task? Or are you still working on it for this iteration?
I don't see how we can ship it with the button being there because it would be disabled at all times and adding the functionality to support the button would risk missing the deadline.
We could add the button in a follow up task, but don't like the idea of adding an additional api call to fetch device details for each device present on the page.
@joepavitt should weigh in on the follow-up task
Can confirm, on production, the editor is disabled, even though this device is in Developer Mode.
Removal for now is fine, and I'll open a follow up task/bug. Although, actually that device is offline, so that's also a reason why it may have been disabled.
For the "X More" button too, I think it would look better as justify-content: space-between
, with the chevron on the right-side
Also also, found it odd to look at and think it's because we're missing the "Instances" (or devices when appropriate) headers even when we don't have any in an application, adding it in feels a little cleaner/more consistent, and I think it makes the "The application has no..." message:
CSS for the application panel seems to have broken as there is no bottom-left or bottom-right border on the cells
I'm not sure what I'm supposed to be looking at, I can't see the difference between the image you provided, the pr and what's on production
The white background for the Application name and summary boxes has changed to grey-300, rather than the white in the existing (and requested) designs.
Didn't notice that, I fixed it
Why include the label for the device status, when the design doesn't show it, and the instances don't have it? I know we have more room to play with, but it adds inconsistency in appearance and looks odd
It was a quick fix to the fact that we don't have icons for all statuses a device can find itself in
For the "X More" button too, I think it would look better as justify-content: space-between, with the chevron on the right-side
done
Also also, found it odd to look at and think it's because we're missing the "Instances" (or devices when appropriate) headers even when we don't have any in an application, adding it in feels a little cleaner/more consistent, and I think it makes the "The application has no..." message:
done
Scope Creep
Am doing my best to keep it under control. The biggest changes were the extraction of device/instance functionality into mixins so I won't repeat existing code and splitting the massive application component into smaller components
I'm not sure what I'm supposed to be looking at, I can't see the difference between the image you provided, the pr and what's on production
On production, and in my design:
Just made some minor CSS amendments to clean up the structure a little:
ml-1
element that was pushing the icon in the status pill off-centreHave also just added link :hover behaviour for the "More" and instance/device tiles too
Description
Alter the applications view to make it more compact
Did not delete the previous application view styling & components. The previous components are interchangeable with the new ones, one could easily add a switch on top of the application view to toggle between the compact and wide application views.
We currently do not have or I couldn't find a device status component that would render icons instead of the status text so I defaulted to the existing device status pill for the time being. This can be sorted out in a follow up task.
Related Issue(s)
closes #4015
Checklist
flowforge.yml
?FlowFuse/helm
to update ConfigMap TemplateFlowFuse/CloudProject
to update values for Staging/ProductionLabels
area:migration
label