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

Use dashboard navbar in the file editor #3646

Merged
merged 8 commits into from
Jul 8, 2024
Merged

Use dashboard navbar in the file editor #3646

merged 8 commits into from
Jul 8, 2024

Conversation

ashton22305
Copy link
Contributor

Fixes #1864

Uses the dashboard navbar by using the application layout for the editor. As a result, configurations needed to be moved to a side panel.

image

I'm not sure if this is the best layout for these options - maybe they would be better placed above or below the editor?

These changes also cause alerts to appear in the editor, which differs from existing functionality.

@johrstrom
Copy link
Contributor

That layout seems fine to me in the sense that it's a panel to the left. Tests are failing because we need to reset the page title for those pages.

expect(browser.title).to eq('File Editor - Open OnDemand - .bashrc')
expect(browser.title).to eq('Dashboard - Open OnDemand')
Copy link
Contributor

Choose a reason for hiding this comment

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

No we shouldn't change the test, we should change the application. Page titles are actually very important for blind users to indicate what that page is all about as they go through their open tabs.

@@ -1,5 +1,239 @@
<div class="d-flex flex-row flex-grow-1">
<div class="flex-sm-column card mx-2">
Copy link
Contributor

Choose a reason for hiding this comment

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

Dont' know how big your screen is - but this renders very large for me (half the page) - I'm guessing because the filename is so long.

image

@ashton22305
Copy link
Contributor Author

Though the text wraps, it seems to break on any character. Is there a way to break only on '/' characters?
image

Copy link
Contributor

@johrstrom johrstrom left a comment

Choose a reason for hiding this comment

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

This works for now, though I think it can still be enhanced. Maybe the left panel is collapsible? IDK - it just takes up a lot of real estate.

@johrstrom johrstrom merged commit eb967c0 into master Jul 8, 2024
23 checks passed
@johrstrom johrstrom deleted the editor-navbar branch July 8, 2024 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

extend nabvar to file editor
3 participants