Add flag to render only job name #402

kvashchuka · 2023-07-31T12:50:26Z

When displaying action statuses of just one repo, it is unnecessary to display the repo name. The "Show job name only" switch will give a user an option to hide repo name and workflow name, leaving only the job name, thus increasing readability.

boring-cyborg · 2023-07-31T12:50:30Z

Thanks for opening this pull request! Please check out our contributing guidelines!

sumanmaity1234

If you are changing the switch label from Show job name only to Show only job name, then change the variable name accordingly.
Add unit tests for your changes
Use conventional commit types for commit messages. Refer https://github.com/otto-de/gitactionboard/blob/main/CONTRIBUTING.md#types

sumanmaity1234 · 2023-07-31T17:14:43Z

frontend/src/components/Preferences.vue

+ <v-card-item class="pt-0 pb-0">
+ <v-switch
+ v-model="showJobNameOnly"
+ label="Show job name only"


Maybe you can change it to "Show only job name"

sumanmaity1234 · 2023-07-31T17:15:13Z

frontend/src/components/Preferences.vue

@@ -99,7 +108,9 @@ export default {
 enableMaxIdleTimeOptimization: preferences.enableMaxIdleTimeOptimization,
 themeInstance,
 isDirty: false,
- showBuildsDueToTriggeredEvents: getShowBuildsDueToTriggeredEvents()
+ showBuildsDueToTriggeredEvents: getShowBuildsDueToTriggeredEvents(),
+ nameTokens: preferences.nameTokens,


is this required? I haven't found any usage of it

sumanmaity1234 · 2023-07-31T17:21:28Z

frontend/src/components/WorkflowDashboard.vue

+ const parts = data.name.split(' :: ');
+ return {
+ ...data,
+ name: preferences.showJobNameOnly ? parts[parts.length - 1] : data.name
+ };


It'll be better to pull out this as mapper. Depends on the configuration use required mapper.

Something like:

const jobNameMapper = (data) => { const parts = data.name.split(' :: '); return { ...data, name: parts[parts.length - 1] } } const identityMapper = data => data; const mapper = preferences.showJobNameOnly ? jobNameMapper : identityMapper;

sumanmaity1234 · 2023-07-31T17:22:02Z

frontend/src/services/apiService.js

+export const fetchConfig = (isMockEnv = false) =>
+ isMockEnv ? Promise.resolve({}) : fetchJsonContent(preparePath('/config'));


Why is this changes required?

running locally with mock data didn't work without this change, but you are write it is out of scope for this PR, will remove it

I am not sure, have you tried the command mentioned on https://github.com/otto-de/gitactionboard/blob/main/CONTRIBUTING.md#run-only-frontend-with-mock-data ?

Co-authored-by: Anna Kvashchuk <[email protected]>

kvashchuka · 2023-08-02T08:41:19Z

Hi @sumanmaity1234 Thanks a lot for your review! We have now addressed all the comments, and made requested changes.

sumanmaity1234

Hi @kvashchuka, I provided a small comment, could you please take care of it?

sumanmaity1234 · 2023-08-03T15:58:29Z

frontend/src/components/WorkflowDashboard.vue

@@ -50,13 +50,24 @@ export default {
 isIdleHealthyBuild(lastBuildStatus, activity) {
 return lastBuildStatus === 'Success' && activity === 'Sleeping';
 },
+ mapNameIfPreferred(data) {
+ const parts = data.name.split(' :: ');


Can we split the name only if required

sumanmaity1234 · 2023-08-03T16:07:07Z

Hi @kvashchuka, I was thinking about this feature. The current solution really works well for single repository but once user has multiple repository, this solution might not add much value.
Therefor I was thinking will it be possible for you to extend the solution to work with multiple repository as well?

One idea could be, allow user to configure which part they are interested, some might want to hide the workflow name, some might repository or combinations of it.

aronhoyer · 2023-08-04T09:55:22Z

hi @sumanmaity1234,

as @kvashchuka is gone for a few weeks, they've asked if I could pick up the mantle on this.

I'll look into implementing your suggestion. however, would it be possible for me to use this feature to look into a new layout for the cards?

sumanmaity1234 · 2023-08-04T10:05:09Z

Hi @aronhoyer, sure, feel free to update the design.
But before you put lot to effort to change it every where maybe you can post the design here

aronhoyer · 2023-08-04T12:30:12Z

did a few quick explorations. I feel like this direction would eliminate this entire pr, but overall a better direction

sumanmaity1234 · 2023-08-04T13:16:04Z

Hi @aronhoyer, overall design looks interesting but couple of things I think you overlook

The name is consists of [Repository name] :: [Workflow Name] :: [Job Name], we don't have branch name
For some users, Name can be really long, therefor I am not sure if this design will work for all the users
For workflow dashboard, we have possibility to hide or show the card/job
Incase of build failure, we display for how long it's failing

Addition to all the above mentioned points, we need to make sure that good numbers of cards/jobs are displayed on the screen without scrolling, so we need to choose height and width accordingly.

If the above points can be easily incorporated in the design then it'll really nice from the product point of view

aronhoyer · 2023-08-04T13:33:38Z

just a general note: all four rows explore different alternatives to the card design and how to display the job status. the designs would not appear at the same time. not sure whether that was unclear or not.

The name is consists of [Repository name] :: [Workflow Name] :: [Job Name], we don't have branch name

yeah I meant to write repo name instead of branch name 😅 that's my b

For some users, Name can be really long, therefor I am not sure if this design will work for all the users

that's why I thought splitting up the parts that make up the formatted name would be beneficial. we could also do things like truncating the job name if it exceeds two lines or something.

For workflow dashboard, we have possibility to hide or show the card/job

not a problem at all. maybe, to not clutter the ui too much if there are many jobs displayed on the dashboard, the show/hide button could appear once the user hovers the card?

Incase of build failure, we display for how long it's failing

not a problem either

sumanmaity1234 · 2023-08-04T15:34:46Z

that's why I thought splitting up the parts that make up the formatted name would be beneficial. we could also do things like truncating the job name if it exceeds two lines or something.

I think it'll good to keep the job name as it is (at least for the first try) and maybe we can truncate the repo name and workflow name.

not a problem at all. maybe, to not clutter the ui too much if there are many jobs displayed on the dashboard, the show/hide button could appear once the user hovers the card?

Not sure how it'll work as somehow we have make it prominent that on click on the card it'll open on GitHub.

aronhoyer · 2023-08-04T21:33:53Z

created a Figma prototype https://www.figma.com/proto/kain0iyc8Cf0zkbV5QoOD6/Untitled?page-id=0%3A1&type=design&node-id=20-83&viewport=238%2C401%2C0.71&t=qNZ3OIAsKstmRDla-1&scaling=min-zoom&mode=design

Not sure how it'll work as somehow we have make it prominent that on click on the card it'll open on GitHub.

good shout. I guess the card should have some hover effect

sumanmaity1234 · 2023-08-05T08:00:23Z

Hi @aronhoyer,
The design looks good to me. But I have the following questions -

Are you using yellow color to represent in progress job?

How are you planning to indicate the last run status?

aronhoyer · 2023-09-11T12:53:04Z

@sumanmaity1234 hi! sorry I went awol for a bit. fell ill and this kinda fell to the wayside

Are you using yellow color to represent in progress job?

that was the idea. either that or a looping progress bar type animation.

How are you planning to indicate the last run status?

won't that just be the first card in the list?

sumanmaity1234 · 2023-09-11T16:14:02Z

Hi @aronhoyer, I hope you are better now.

that was the idea. either that or a looping progress bar type animation.

Sure, that make sense. I just wanted to clarify my doubt.

won't that just be the first card in the list?

Unable to understand what do you meant by "first card". In the dashboard, you'll have single grid/box for each job.

My question was related to the last run status for a job. For example, in the following image, you can see the current jobs are running but it also indicate the last run status, for job 1 (webpack-dev-server :: Cancel :: Cancel Previous Runs), it's passed and for job2 (webpack-dev-server :: webpack-dev-server :: Test - windows-latest - Node v16.x, Webpack latest) it's failed

Note: screenshot is from https://otto-de.github.io/gitactionboard/#/workflow-jobs

kvashchuka · 2023-10-31T14:28:27Z

Hi @sumanmaity1234! I think this PR lost traction as the scope grew larger. Could we please go back a step and reconsider merging the PR with the original intent (aka simply have a flag to only show the job name)? I do understand that it is not a perfect solution for every user, but it makes a big difference for the usage of gitaction board for my team and I believe it does not have any negative impact on other users.

sumanmaity1234 · 2024-02-04T10:08:36Z

Hi @kvashchuka, could you please check if your changes works with the latest codebase? Also please add a screenshot how will it look post your changes.

Additionally, there are couple of code change requested.

Once these changes are done, I'll merge the PR

Thanks

boring-cyborg bot added the javascript Changes related to frontend label Jul 31, 2023

sumanmaity1234 requested changes Jul 31, 2023

View reviewed changes

kvashchuka force-pushed the main branch from 6b79af7 to 93111af Compare August 1, 2023 14:11

feat: add flag to render only job name

cbcbd03

Co-authored-by: Anna Kvashchuk <[email protected]>

kvashchuka force-pushed the main branch from 93111af to cbcbd03 Compare August 2, 2023 08:38

sumanmaity1234 requested changes Aug 3, 2023

View reviewed changes

sumanmaity1234 added the enhancement New feature or request label Aug 5, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add flag to render only job name #402

Add flag to render only job name #402

kvashchuka commented Jul 31, 2023

boring-cyborg bot commented Jul 31, 2023

sumanmaity1234 left a comment

sumanmaity1234 Jul 31, 2023

sumanmaity1234 Jul 31, 2023

sumanmaity1234 Jul 31, 2023

sumanmaity1234 Jul 31, 2023

kvashchuka Aug 1, 2023

sumanmaity1234 Aug 3, 2023

kvashchuka commented Aug 2, 2023

sumanmaity1234 left a comment

sumanmaity1234 Aug 3, 2023

sumanmaity1234 commented Aug 3, 2023

aronhoyer commented Aug 4, 2023

sumanmaity1234 commented Aug 4, 2023

aronhoyer commented Aug 4, 2023

sumanmaity1234 commented Aug 4, 2023

aronhoyer commented Aug 4, 2023

sumanmaity1234 commented Aug 4, 2023

aronhoyer commented Aug 4, 2023

sumanmaity1234 commented Aug 5, 2023 •

edited

aronhoyer commented Sep 11, 2023

sumanmaity1234 commented Sep 11, 2023

kvashchuka commented Oct 31, 2023 •

edited

sumanmaity1234 commented Feb 4, 2024

		export const fetchConfig = (isMockEnv = false) =>
		isMockEnv ? Promise.resolve({}) : fetchJsonContent(preparePath('/config'));

Add flag to render only job name #402

Are you sure you want to change the base?

Add flag to render only job name #402

Conversation

kvashchuka commented Jul 31, 2023

boring-cyborg bot commented Jul 31, 2023

sumanmaity1234 left a comment

Choose a reason for hiding this comment

sumanmaity1234 Jul 31, 2023

Choose a reason for hiding this comment

sumanmaity1234 Jul 31, 2023

Choose a reason for hiding this comment

sumanmaity1234 Jul 31, 2023

Choose a reason for hiding this comment

sumanmaity1234 Jul 31, 2023

Choose a reason for hiding this comment

kvashchuka Aug 1, 2023

Choose a reason for hiding this comment

sumanmaity1234 Aug 3, 2023

Choose a reason for hiding this comment

kvashchuka commented Aug 2, 2023

sumanmaity1234 left a comment

Choose a reason for hiding this comment

sumanmaity1234 Aug 3, 2023

Choose a reason for hiding this comment

sumanmaity1234 commented Aug 3, 2023

aronhoyer commented Aug 4, 2023

sumanmaity1234 commented Aug 4, 2023

aronhoyer commented Aug 4, 2023

sumanmaity1234 commented Aug 4, 2023

aronhoyer commented Aug 4, 2023

sumanmaity1234 commented Aug 4, 2023

aronhoyer commented Aug 4, 2023

sumanmaity1234 commented Aug 5, 2023 • edited

aronhoyer commented Sep 11, 2023

sumanmaity1234 commented Sep 11, 2023

kvashchuka commented Oct 31, 2023 • edited

sumanmaity1234 commented Feb 4, 2024

sumanmaity1234 commented Aug 5, 2023 •

edited

kvashchuka commented Oct 31, 2023 •

edited