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

Some additions to the project import configuration page #185

Merged
merged 2 commits into from
Jul 11, 2023

Conversation

agjohnson
Copy link
Contributor

@agjohnson agjohnson commented Jul 7, 2023

I pulled down #166 to provide a bit of guidance, but ended up with fixes
to the PR either way. I added a few features on top, to replace plain
text with visual elements and interactive components. These aren't fully
working yet (the example selector namely, see #184).

image

I pulled down #166 to provide a bit of guidance, but ended up with fixes
to the PR either way. I added a few features on top, to replace plain
text with visual elements and interactive components. These aren't fully
working yet (the example selector namely, see #184).
@agjohnson agjohnson requested a review from a team as a code owner July 7, 2023 01:02
@agjohnson agjohnson requested a review from humitos July 7, 2023 01:02
agjohnson added a commit that referenced this pull request Jul 7, 2023
- Make the search more obvious it is a search
- Drop the mention of syncing repositories down to the bottom of the
  page. This should be a last resort in most cases, but still needed UX
- Tune the copy to point the user to "search for a project", instead of
  instructions on not finding a repository
- Tune some of the dynamic elements used in the search results
- Some changes related to #166 and #185
@agjohnson
Copy link
Contributor Author

I was able to make use of the column structure from the base template, in #186. This makes review here messy as I don't want to yet rebase this and the underlying PR on top of that. I'll probably open up another PR based off that and merge in parts of #166 to that branch.

Copy link
Member

@humitos humitos left a comment

Choose a reason for hiding this comment

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

This looks good to me 💯

I find really hard to understand what are the CSS classes that I should use, but I guess that will come eventually.

One thing that I noticed is that it's hard to realize the content of the suggested config file is scrollable because there is no vertical scroll bar when the page is shown. So, it's easy to think the file ends there when it's not. I'd like to show as much as possible of its content without hiding the form buttons if that's possible.

Another thing I noticed is that for some reason the YAML content is not highlighted as other code format is. Is it possible to highlight it without too much work?

@agjohnson
Copy link
Contributor Author

I find really hard to understand what are the CSS classes that I should use, but I guess that will come eventually.

Yeah, it's not obvious what elements or element variations are being used until you start working with FUI/SUI more. I noted on the other PR that I'll cover this more with:

But it's also always harder building new UI or more custom new UI because what element works best is always more subjective.

One thing that I noticed is that it's hard to realize the content of the suggested config file is scrollable

Yeah, this is a bit of a browser thing, but we might be able to use a more native scroll bar here instead. Most browsers show a micro version of the scroll bar these days though.

I'd like to show as much as possible of its content without hiding the form buttons if that's possible

I didn't do this because showing the full source without scrolling pushes the buttons well out of the viewport, and then it isn't clear what the user is supposed to do with the page -- it doesn't look like a form. The scrolling segment isn't a great solution, but it's better in this regard.

Ultimately, I'd see us using a popup modal for the example source instead, with the code examples popping up after the user selects one. This gives us a chance to provide more context on the page around the examples, and make it really obvious what the user is supposed to do with the example source. For a first pass, I think the scrolling segment is a good compromise.

Another thing I noticed is that for some reason the YAML content is not highlighted as other code format is.

It's because we don't have Pygments available, the other UIs perform the Pygments highlighting inside the view code. We do need to show the actually plain text source though, where Pygments does alter the source to add the HTML classes, so I'd probably lean towards using something like highlight.js or another JS library.

agjohnson added a commit that referenced this pull request Jul 11, 2023
* Tweak project create layout and elements

- Make the search more obvious it is a search
- Drop the mention of syncing repositories down to the bottom of the
  page. This should be a last resort in most cases, but still needed UX
- Tune the copy to point the user to "search for a project", instead of
  instructions on not finding a repository
- Tune some of the dynamic elements used in the search results
- Some changes related to #166 and #185

* Unify project creation columns and layout

Avoid switching layouts multiple times, always use the same two columns.
This is more predictable for the user and for us.

* Typo fixes

Co-authored-by: Manuel Kaufmann <humitos@gmail.com>

---------

Co-authored-by: Manuel Kaufmann <humitos@gmail.com>
@agjohnson agjohnson merged commit 3d33c89 into humitos/import-wizard-config Jul 11, 2023
agjohnson added a commit that referenced this pull request Jul 11, 2023
* Some additions to the project import configuration page

I pulled down #166 to provide a bit of guidance, but ended up with fixes
to the PR either way. I added a few features on top, to replace plain
text with visual elements and interactive components. These aren't fully
working yet (the example selector namely, see #184).

* Update JS comment to be accurate
agjohnson added a commit that referenced this pull request Jul 11, 2023
* Import project: suggest a config file (yaml)

I tried to add the intermediate page where we suggest a common Sphinx YAML file
for the user to copy and paste.

I found the pattern a little hard to follow. The base template didn't have all
the blocks I needed, so I added some more but I don't feel comfortable with
them.

I also had to create a block to overwrite the CSS classes of the main div, which
sounds weird to me as well.

Besides, I'm not sure how to use the CSS classes to achieve what I need: syntax
highlighting, smaller text, etc.

* Some additions to the project import configuration page (#185)

* Some additions to the project import configuration page

I pulled down #166 to provide a bit of guidance, but ended up with fixes
to the PR either way. I added a few features on top, to replace plain
text with visual elements and interactive components. These aren't fully
working yet (the example selector namely, see #184).

* Update JS comment to be accurate

---------

Co-authored-by: Anthony <aj@ohess.org>
@stsewd stsewd deleted the agj/import-wizard-config branch July 24, 2023 17:10
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