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.

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

Bureau landing page and Menu TM-1855 #1006

Merged
merged 8 commits into from
Jul 20, 2020
Merged

Conversation

elizabeth-jimenez
Copy link

No description provided.

Comment on lines 113 to 213
getLogs: PropTypes.func,
getLogsList: PropTypes.func,
isLoading: PropTypes.bool,
logs: PropTypes.string,
logsIsLoading: PropTypes.bool,
logsHasErrored: PropTypes.bool,
logsList: PropTypes.arrayOf(PropTypes.string),
logsListIsLoading: PropTypes.bool,
logsListHasErrored: PropTypes.bool,
log: PropTypes.arrayOf(PropTypes.string),
logIsLoading: PropTypes.bool,
logHasErrored: PropTypes.bool,
logToDownload: PropTypes.string,
logToDownloadIsLoading: PropTypes.bool,
logToDownloadHasErrored: PropTypes.bool,
getLog: PropTypes.func,
getLogToDownload: PropTypes.func,
getSyncJobs: PropTypes.func,
syncJobs: PropTypes.arrayOf(PropTypes.shape({})),
syncJobsIsLoading: PropTypes.bool,
putAllSyncJobs: PropTypes.func,
putAllSyncsIsLoading: PropTypes.bool,
patchSyncIsLoading: PropTypes.bool,
patchSyncJob: PropTypes.func,
patchSyncHasErrored: PropTypes.bool,
getUsers: PropTypes.func,
getTableStats: PropTypes.func,
totalUsers: PropTypes.oneOfType([PropTypes.arrayOf(PropTypes.shape({})),
PropTypes.shape({ count: PropTypes.number })]),
fetchFeatureFlagsData: PropTypes.func,
featureFlags: PropTypes.shape({}),
};

BureauContainer.defaultProps = {
getLogs: EMPTY_FUNCTION,
getLogsList: EMPTY_FUNCTION,
isLoading: false,
logs: '',
logsIsLoading: false,
logsHasErrored: false,
logsList: [],
logsListIsLoading: false,
logsListHasErrored: false,
log: [],
logIsLoading: false,
logHasErrored: false,
logToDownload: '',
logToDownloadIsLoading: false,
logToDownloadHasErrored: false,
getLog: EMPTY_FUNCTION,
getLogToDownload: EMPTY_FUNCTION,
getSyncJobs: EMPTY_FUNCTION,
syncJobs: [],
syncJobsIsLoading: false,
putAllSyncsIsLoading: false,
putAllSyncJobs: EMPTY_FUNCTION,
patchSyncIsLoading: false,
patchSyncJob: EMPTY_FUNCTION,
patchSyncHasErrored: false,
getUsers: EMPTY_FUNCTION,
getTableStats: EMPTY_FUNCTION,
totalUsers: {},
fetchFeatureFlagsData: EMPTY_FUNCTION,
featureFlags: {},
};

const mapStateToProps = state => ({
logs: state.logsSuccess,
logsIsLoading: state.logsIsLoading,
logsHasErrored: state.logsHasErrored,
logsList: state.logsListSuccess,
logsListIsLoading: state.logsListIsLoading,
logsListHasErrored: state.logsListHasErrored,
log: state.logSuccess,
logIsLoading: state.logIsLoading,
logHasErrored: state.logHasErrored,
logToDownload: state.logToDownloadSuccess,
logToDownloadIsLoading: state.logToDownloadIsLoading,
logToDownloadHasErrored: state.logToDownloadHasErrored,
syncJobs: state.syncs,
syncJobsIsLoading: state.syncsIsLoading,
putAllSyncsIsLoading: state.putAllSyncsIsLoading,
patchSyncIsLoading: state.patchSyncIsLoading,
patchSyncHasErrored: state.patchSyncHasErrored,
totalUsers: state.usersSuccess,
featureFlags: state.featureFlags,
});

export const mapDispatchToProps = dispatch => ({
getLogs: () => dispatch(getLogs()),
getLogsList: () => dispatch(getLogsList()),
getLog: id => dispatch(getLog(id)),
getLogToDownload: id => dispatch(getLogToDownload(id)),
getSyncJobs: () => dispatch(syncsFetchData()),
putAllSyncJobs: () => dispatch(putAllSyncs()),
patchSyncJob: data => dispatch(patchSync(data)),
getUsers: () => dispatch(getUsers()),
getTableStats: () => dispatch(getTableStats()),
fetchFeatureFlagsData: () => dispatch(fetchFeatureFlagsData()),
});

Choose a reason for hiding this comment

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

Let's remove all the copied state values and actions

Copy link
Author

Choose a reason for hiding this comment

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

Oops! Missed those, thanks! Will do!

import { syncsFetchData, putAllSyncs, patchSync } from '../../actions/synchronizations';
import { EMPTY_FUNCTION } from '../../Constants/PropTypes';

export const downloadFile = (text) => {

Choose a reason for hiding this comment

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

Let's also remove all the copied over functions

Copy link
Author

Choose a reason for hiding this comment

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

For sure, will do :)

],
},
{
text: 'Position Manager',

Choose a reason for hiding this comment

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

Let's increase the width of the menu so that this isn't so close to the edge

Screen Shot 2020-07-14 at 9 49 43 AM

// onSelectOption={myFunction}
/>
</div>
<div className="usa-width-one-whole">

Choose a reason for hiding this comment

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

I think if we end up going with data viz here, it will be abstracted out to a data viz library. I think this is valuable to keep for demo purposes, but we will need to wrap it in a static content feature flag so that it doesn't appear in the DOS environment

Copy link
Author

Choose a reason for hiding this comment

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

Hopefully D3! For sure, I think it helps get the imagination going. Ok, will do :)

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

nice!

@@ -169,6 +169,50 @@ export const GET_PROFILE_MENU = () => MenuConfig([
},
],
},
{
text: 'Bureau',
route: '/profile/bureau/',

Choose a reason for hiding this comment

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

If you change this to route: '/profile/bureau/dashboard/',, it will be clickable in the condensed menu

Copy link
Author

Choose a reason for hiding this comment

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

changed to route: '/profile/bureau/positionlists/' since that's going to be our "default" for now. Didn't really see a change tho :/

@mjoyce91
Copy link

Looks good. Overall comments:

  • Let's remove any copy 🍝 that was carried over
  • Let's use the static content feature flag for things we won't be showing in our first pass of bureau functionality

@elizabeth-jimenez
Copy link
Author

Looks good. Overall comments:

  • Let's remove any copy 🍝 that was carried over
  • Let's use the static content feature flag for things we won't be showing in our first pass of bureau functionality

Do you think I should create a feature flag for each menu item, or one for the whole Bureau menu, or both?

@mjoyce91
Copy link

@elizabeth-jimenez

Do you think I should create a feature flag for each menu item, or one for the whole Bureau menu, or both?

Ultimately that Bureau landing page won't be used in the first pass. I think what I'd do is set the "default" view to the Positions List since that's the first bit of functionality we'll be completing. Then just feature flag the menu links that we won't use with the existing static_content flag.

@elizabeth-jimenez
Copy link
Author

@elizabeth-jimenez

Do you think I should create a feature flag for each menu item, or one for the whole Bureau menu, or both?

Ultimately that Bureau landing page won't be used in the first pass. I think what I'd do is set the "default" view to the Positions List since that's the first bit of functionality we'll be completing. Then just feature flag the menu links that we won't use with the existing static_content flag.

Gotcha! Will do :)

@elizabeth-jimenez
Copy link
Author

image
😍

🙈 dont check on small screen!

@elizabeth-jimenez elizabeth-jimenez changed the title Bureau landing page and Menu Bureau landing page and Menu TM-1855 Jul 15, 2020
@mjoyce91 mjoyce91 merged commit be57706 into dev Jul 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants