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

feat: New Course Comparison Dashboard #938

Merged
merged 5 commits into from
Sep 16, 2024

Conversation

saraburns1
Copy link
Contributor

@saraburns1 saraburns1 marked this pull request as ready for review September 11, 2024 16:50
Copy link
Contributor

@bmtcril bmtcril left a comment

Choose a reason for hiding this comment

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

Just one set of questions, 3 times :)

color_pn: false
column_config:
active_count:
columnWidth: 200
Copy link
Contributor

Choose a reason for hiding this comment

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

These aren't usually present, and seem to keep the columns from resizing when the screen is smaller. Any idea why they're here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, that was a conscious decision to prevent the 'tags' column from taking over the whole space and squishing the rest of the columns together

Copy link
Contributor

Choose a reason for hiding this comment

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

Will "tags" wrap if we put spaces after the commas?

Copy link
Contributor Author

@saraburns1 saraburns1 Sep 13, 2024

Choose a reason for hiding this comment

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

it will, but if we dont specify the width for the other columns, itll still push all the columns to the left
image

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm yeah I see.

@Ian2012 can we get the course comparison dashboard embedded on the sandbox site?
@crathbun428 do we have any guidance for the resolutions / page sizes we want to support?

I know we're not currently worrying about mobile, but I think we may need to adjust the sizes to make things fit reasonably on, say, a laptop screen when embedded. Does that sound fair?

Choose a reason for hiding this comment

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

This does sound fair - I did not think about this, but can come up with some requirements for this ASAP.

Copy link
Contributor

Choose a reason for hiding this comment

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

For sure, I think this is non-blocking for this PR and we can clean up the specific sizes later.

@saraburns1 saraburns1 merged commit 282ba9b into openedx:main Sep 16, 2024
9 checks passed
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.

3 participants