Skip to content

Adding export schedule as class button#48

Open
lieutenantlin wants to merge 1 commit into
mainfrom
export
Open

Adding export schedule as class button#48
lieutenantlin wants to merge 1 commit into
mainfrom
export

Conversation

@lieutenantlin
Copy link
Copy Markdown
Collaborator

Making a button to export schedule as classes or ids

Signed-off-by: FordoA455 <milobylin@gmail.com>
@lieutenantlin lieutenantlin added the enhancement New feature or request label May 13, 2024
@lieutenantlin lieutenantlin requested a review from maxstrid May 13, 2024 20:28
@lieutenantlin lieutenantlin self-assigned this May 13, 2024
@lieutenantlin
Copy link
Copy Markdown
Collaborator Author

@maxstrid can you take a look at the layout of the export buttons? There might need to be some changes to spacing to make it look better

Comment thread src/Schedule/Schedule.tsx
for (let i: number = 0; i < highestLength; i++) {
const row: ScheduleExportPeriodIds = {
const idRow: ScheduleExportPeriodIds = {
period1ClassIds: periods[0]["classes"][i] == undefined ? "" : periods[0]["classes"][i]['id'],
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

These don't need to be separate types, make it one type and just export class or id depending on the option

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Same here

Comment thread src/Schedule/Schedule.tsx
<span>Export</span>
</CSVLink>
<div className='flex flex-row m-auto'>
<CSVLink
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

The spacing is fine here but I'd honestly prefer this to be a submenu if you could, or a modal which shows up and lets you choose between classes and ids. Or it might just be better to include the ids in the export as well as class names.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Can be resolved in #50, but just for the meeting with Ms. Morelli, it is fine

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants